-----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

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