fix for some tbb test suite issues ?

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi, Recently, I had some issues when running the TBB test suite. I could submit a patch if you agree that the following can/should be improved: 1-) Make it possible to pass newlayout=0 from the command line and add documentation for the "newlayout" option. The error messages I got due to the "newlayout" option was very hard to trace. 2-) The description for the "enable-tests" option says" "Only run the list of tests selected" This is not completely true since 'tor_bootstrap' will run regardless of that option (managed by the property "always => 1".) We can either fix the doc or ignore "always=1" when the "enable-tests" option is set. 3-) When the suite skips a test due to "enable-tests", it should print out smt. like "Skipping the test..." or shouldn't print the test name at all. Currently if prints the following even though the tests were skipped: ********************** * Running test check * ********************** ********************************* * Running test https-everywhere * ********************************* ****************************************** * Running test https-everywhere-disabled * ****************************************** Best, Gunes -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQEcBAEBAgAGBQJT8s53AAoJEPb7JcMmVt4gpHoIANGTFSfd+m2AIHx2CJ5awpjh 8lSDxLTOB/+kZ/pKYEfa3EEaVZYeW79Kml0vj4iyHsYdE5fRVxgNNFdk1T8Uotot 719BFRhl2vBVWsGX2w2wkGlzVpF1fwG6EtDcQmxRfuerK52cUuNrMbgel3FGO2OD BZe08PPdmZ7KStoEpbLj7jhoqYvuaf29EySHgkVwTHQgL3E3Sz9CtVuFGR9+sFkS avupQ1z/v8NupC/SqDX0Bclfo5ypbG4hMaQVcirFmVs6EMG+7CxIYBaDl5nmADdw 2Nq3quQTnbpvWiatMU2Jas6/48Q7wacvGCW8WwKNwILPYOfN6Nbr0cvm5AaiRPA= =OcwV -----END PGP SIGNATURE-----

On Mon, 18 Aug 2014, Gunes Acar wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
Recently, I had some issues when running the TBB test suite. I could submit a patch if you agree that the following can/should be improved:
1-) Make it possible to pass newlayout=0 from the command line and add documentation for the "newlayout" option.
There is a --no-newlayout command line option: https://people.torproject.org/~boklm/tbbtests-doc/usage.html I noticed that I forgot to regenerate the html version of the doc after it was changed, so maybe it is the reason you didn't see it.
The error messages I got due to the "newlayout" option was very hard to trace.
Indeed, I remember having the same problem previously, and the error message in this case was not very helpful. It could be improved.
2-) The description for the "enable-tests" option says" "Only run the list of tests selected"
This is not completely true since 'tor_bootstrap' will run regardless of that option (managed by the property "always => 1".)
We can either fix the doc or ignore "always=1" when the "enable-tests" option is set.
In the begining, all the tests depended on 'tor_bootstrap' being run, so I added the "always => 1" property to avoid listing it in 'enable-tests' everytime. Now that we have other tests that don't require 'tor_bootstrap', this property has become a little annoying, so maybe it could just be removed. An other possible improvement could be to add support for dependencies between tests, so that when a test is enabled, its dependencies are enabled too. But just removing the 'always' property would probably be good enough for now.
3-) When the suite skips a test due to "enable-tests", it should print out smt. like "Skipping the test..." or shouldn't print the test name at all. Currently if prints the following even though the tests were skipped:
This would be a good change. Thanks for reporting those problems. Patches for any of those issues are welcome. Let me know if you need some help. Nicolas

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi Nicolas, Here's the patch: https://github.com/gunesacar/tbb-fp-tests/blob/master/always.patch It removes the check for the "always" option and prevents printing of the test names that are not enabled. A new corner case was: when you run some tests without running the tor_bootstrap, stop_tor (TorBootstrap.pm) gets called before running start_tor in the first place. Then it (stop_tor) tries to kill a non-existing Tor process and gives the following error: "Can't kill a non-numeric process ID..." So I added a check for the tor pid to fix this. Hope that's all fine, please check the patch since I don't know Perl at all. Cheers, Gunes On 08/19/2014 02:51 AM, Nicolas Vigier wrote:
On Mon, 18 Aug 2014, Gunes Acar wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi,
Recently, I had some issues when running the TBB test suite. I could submit a patch if you agree that the following can/should be improved:
1-) Make it possible to pass newlayout=0 from the command line and add documentation for the "newlayout" option.
There is a --no-newlayout command line option: https://people.torproject.org/~boklm/tbbtests-doc/usage.html
I noticed that I forgot to regenerate the html version of the doc after it was changed, so maybe it is the reason you didn't see it.
The error messages I got due to the "newlayout" option was very hard to trace.
Indeed, I remember having the same problem previously, and the error message in this case was not very helpful. It could be improved.
2-) The description for the "enable-tests" option says" "Only run the list of tests selected"
This is not completely true since 'tor_bootstrap' will run regardless of that option (managed by the property "always => 1".)
We can either fix the doc or ignore "always=1" when the "enable-tests" option is set.
In the begining, all the tests depended on 'tor_bootstrap' being run, so I added the "always => 1" property to avoid listing it in 'enable-tests' everytime. Now that we have other tests that don't require 'tor_bootstrap', this property has become a little annoying, so maybe it could just be removed.
An other possible improvement could be to add support for dependencies between tests, so that when a test is enabled, its dependencies are enabled too. But just removing the 'always' property would probably be good enough for now.
3-) When the suite skips a test due to "enable-tests", it should print out smt. like "Skipping the test..." or shouldn't print the test name at all. Currently if prints the following even though the tests were skipped:
This would be a good change.
Thanks for reporting those problems. Patches for any of those issues are welcome. Let me know if you need some help.
Nicolas
-----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) iQEcBAEBAgAGBQJT89wBAAoJEPb7JcMmVt4gqnAIALHwy3V6g/z6FWRmm5w5Depx 8bpyQXnCKHdMIOc1y1RAlRufbzEsemu0Hm78DR2OkFV/HhOKBXxYsiv4yP4uaOfN hUv/cnXMAvDZ1zfn/xqsfb0YYznlTfvyy80tyUTD0v9IyXXB0u8mHvrd70LrSKFe lB6B7CP5n+IkXpf7tOATh2w1OCb6GNYY2deZGto/D8Fviwdb4rnT93kfZX6m6UXX RvpFuofGfikAGOPC4CLsWrR0XT/XqxM9CmrNTDbF/LUwqJIrQh1QOUZzZYWjSGhB pCnJ6s0MCnKNoxe0ba2vSG3Q6+7uYcATL/S+BACv9GygxeWkhwbwzsd2oszz4lM= =B3eu -----END PGP SIGNATURE-----

On Tue, 19 Aug 2014, Gunes Acar wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi Nicolas,
Here's the patch: https://github.com/gunesacar/tbb-fp-tests/blob/master/always.patch
It removes the check for the "always" option and prevents printing of the test names that are not enabled.
A new corner case was: when you run some tests without running the tor_bootstrap, stop_tor (TorBootstrap.pm) gets called before running start_tor in the first place.
Then it (stop_tor) tries to kill a non-existing Tor process and gives the following error: "Can't kill a non-numeric process ID..."
So I added a check for the tor pid to fix this.
Hope that's all fine, please check the patch since I don't know Perl at all.
Thanks. This looks good. Can you make this in git patches (with git format-patch) ? Nicolas

Here's the same one made with git format-patch: https://github.com/gunesacar/tbb-fp-tests/blob/master/always.patch Best, Gunes On 08/20/2014 03:08 AM, Nicolas Vigier wrote:
On Tue, 19 Aug 2014, Gunes Acar wrote:
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
Hi Nicolas,
Here's the patch: https://github.com/gunesacar/tbb-fp-tests/blob/master/always.patch
It removes the check for the "always" option and prevents printing of the test names that are not enabled.
A new corner case was: when you run some tests without running the tor_bootstrap, stop_tor (TorBootstrap.pm) gets called before running start_tor in the first place.
Then it (stop_tor) tries to kill a non-existing Tor process and gives the following error: "Can't kill a non-numeric process ID..."
So I added a check for the tor pid to fix this.
Hope that's all fine, please check the patch since I don't know Perl at all.
Thanks. This looks good.
Can you make this in git patches (with git format-patch) ?
Nicolas

On Wed, 20 Aug 2014, Gunes Acar wrote:
Here's the same one made with git format-patch: https://github.com/gunesacar/tbb-fp-tests/blob/master/always.patch
Applied. Thanks.
participants (2)
-
Gunes Acar
-
Nicolas Vigier