Bug 2291 - test.py limitations
test.py limitations
Status: PATCH PENDING
Product: ns-3
Classification: Unclassified
Component: core
ns-3-dev
PC Linux
: P5 enhancement
Assigned To: Peter Barnes
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-09 12:18 EST by Matthieu Coudron
Modified: 2016-03-31 18:00 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Matthieu Coudron 2016-02-09 12:18:22 EST
I wonder if others feel the same way about the test.py script but I find it hardly practical (I may have missed some features as well); a few reasons why:
1/ does not seem to take into account NS_LOG in $ NS_LOG="*" ./test.py
2/ I can't display to stdout the detailed result of the tests, I have to use export to html or xml
3/ not possible to run several tests or use a regex on test names. For instance in python3 argparse module, you could have accumulator and for instance write, ./test.py -stest1 -stest2
4/ there is no default flag to run a debugger, memorizing the full command template is kinda hard ? 

For now, I use a hacky wrapper (mixed with some personal) that launches dce or ns3 tests (depending if the test is prefixed with 'dce') to work around some of the limitations but some problems would be better fixed upstream
https://github.com/teto/ns3-testing
Comment 1 Peter Barnes 2016-02-09 14:36:04 EST
[Tom Henderson]

I do not disagree with some of the limitations of that program, but I would put it to "PATCH WANTED" state-- as long as requirements are agreed upon and someone wants to submit a patch (even to replace it altogether), I would consider, but I'm not going to prioritize it personally.  Are you planning to submit patches and work on it?

I'm sort of asking also because it might make a suitable GSOC project idea (part of a project around testing ns-3) if you are not interested and are posting more of a wishlist.
Comment 2 Peter Barnes 2016-02-09 14:36:27 EST
[Matthieu Coudron]

I was really eager to do so but I I would have done so using python3
(for argparse) and I thought it would have been a nogo ? hence I
implemented my wrapper.

The need reappeared recently when updating my gsoc clock branch.
I want want to always run 2 tests and also I was not able to see
precisely the failing tests without opening the 2 html files :/

One of my concern is also how to allow for a better convergence with
DCE (like using the same test.py for both projects etc...).
Comment 3 Peter Barnes 2016-02-09 14:36:54 EST
My take (without giving it much thought) is that test.py is for whole-sale testing of ns-3, not the specialized/advanced uses that you describe.  For advanced use cases, I think one should use test-runner directly, wrapped in your own convenience script if your use is more involved.

Re 1:  NS_LOG is deliberately suppressed in test.py:
   # If lots of logging is enabled, we can crash Python when it tries to 
   # save all of the text.  We just don't allow logging to be turned on when
   # test.py runs.  If you want to see logging output from your tests, you
   # have to run them using the test-runner directly.
Now, whether that’s a *good* reason we can debate :)

Re: 2:  Getting stdout/stderr is straightforward with test-runner (I think it’s --verbose).  I thought the html/txt output of test.py is more geared to generating web-ready test reports.

Re: 3:  Other than running a full test suite, agree there is too little support for wildcarding the tests to run.  I’d put this in test.py, not test-runner.

Re: 4:  I’m not sure a debugger option can be widely supported, since there are many different debuggers with different invocation forms.  Personally I use gdb rarely, ddd more often, and TotalView as well. The command template version provides flexibility to feed your debugger, whatever that happens to be. Again, IMO one’s particular use case can be wrapped in a convenience script.
Comment 4 Tom Henderson 2016-02-09 16:30:51 EST
(In reply to Peter Barnes from comment #3)
> My take (without giving it much thought) is that test.py is for whole-sale
> testing of ns-3, not the specialized/advanced uses that you describe.  For
> advanced use cases, I think one should use test-runner directly, wrapped in
> your own convenience script if your use is more involved.

Another view is that test.py is mainly for active maintainers so we should prioritize making it less painful for them, so if an active maintainer complains that it gets in the way, and others agree, I will be sympathetic.  However, that said, we will never have perfect scripts to suit everyone, so yes, sometimes custom aliases are appropriate.

A similar topic comes up from time to time in replacing Waf.  If someone wants to propose a change, OK, but please 1) maintain all existing functionality that is being used (i.e. get agreement from maintainers before dropping any feature), and 2) agree to maintain the new program going forward.  I would argue the same here; we should patch test.py unless someone wants to both rewrite *and* actively maintain the rewrite.

> 
> Re 1:  NS_LOG is deliberately suppressed in test.py:
>    # If lots of logging is enabled, we can crash Python when it tries to 
>    # save all of the text.  We just don't allow logging to be turned on when
>    # test.py runs.  If you want to see logging output from your tests, you
>    # have to run them using the test-runner directly.
> Now, whether that’s a *good* reason we can debate :)

I understand why this was disabled but it may be too restrictive for a common case in which only one suite is being run.  I would support a patch that would allow NS_LOG to be active if e.g. the user passed a flag to do so.

How about NS_LOG="..." ./test.py --log .... enables logging?

> 
> Re: 2:  Getting stdout/stderr is straightforward with test-runner (I think
> it’s --verbose).  I thought the html/txt output of test.py is more geared to
> generating web-ready test reports.

Yes, ./waf --run "test-runner --suite=<suite> --verbose" will do it.  I suppose that another test.py option such as "-o   write detailed test results to stdout" could be added to simplify the syntax.  This is probably useful because, indeed, sometimes I just want to see what failed and I have to redirect to file to do so.

> 
> Re: 3:  Other than running a full test suite, agree there is too little
> support for wildcarding the tests to run.  I’d put this in test.py, not
> test-runner.

agree; I often have to do "./test.py -l | grep pattern" and then enumerate choices.

> 
> Re: 4:  I’m not sure a debugger option can be widely supported, since there
> are many different debuggers with different invocation forms.  Personally I
> use gdb rarely, ddd more often, and TotalView as well. The command template
> version provides flexibility to feed your debugger, whatever that happens to
> be. Again, IMO one’s particular use case can be wrapped in a convenience
> script.

I'm somewhat neutral on this point since it may be difficult to cover each maintainer's invocation forms.

If we agree on any new desired features, let's split them out into separate issues in PATCH WANTED or ASSIGNED states and for PATCH WANTED, suggest them as student projects.
Comment 5 Matthieu Coudron 2016-02-11 10:37:49 EST
>My take (without giving it much thought) is that test.py is for whole-sale testing of ns-3, not the specialized/advanced uses that you describe.
I disagree here, test.py can run valgrind and generate HTML files (sounds advanced to me) but it can't write to stdout directly which parts of the tests fail (referring to "test-runner --verbose") :s

>For advanced use cases, I think one should use test-runner directly, wrapped in your own convenience script if your use is more involved.
It should provide an interface useful for its main users, which I suppose are ns3 developers. I use tests a lot to develop mptcp and clock features and I believe my workflow is quite conventional (I develop code impacting these files, hence I want to run theses these 2 to 4 tests only and not the fullsuite etc...). Then if a test crash, I want to run a debugger. Maybe I am wrong and people on the mailing list would say differently.. I agree that we don't use the same (I suppose most use GDB). Maybe that could be amendable in .ns3rc, like "which command to run when running ./test --debug" ?

So far I can do most of these with my wrapper but I would to avoid having everyone rewrite the same wrapper.

I found 2 problems with current  logger, if I run:
python3.4 ./test.py -k
Traceback (most recent call last):
  File "./test.py", line 1935, in <module>
    sys.exit(main(sys.argv))
  File "./test.py", line 1932, in main
    return run_tests()
  File "./test.py", line 1021, in run_tests
    read_waf_config()
  File "./test.py", line 578, in read_waf_config
    for line in open(".lock-waf_" + sys.platform + "_build", "rt"):
FileNotFoundError: [Errno 2] No such file or directory: '.lock-waf_linux_build'

The file is called ".lock-waf_linux2_build". Apparently it was created when running ./waf configure (with python2) and my guess is that python2 and python3 don't return the same value for sys.platform.

"

Also even if I solve the previous problem using the same version of python, then the command still returns:
  File "./test.py", line 1935, in <module>     sys.exit(main(sys.argv))   File "./test.py", line 1932, in main     return run_tests()   File "./test.py", line 1173, in run_tests     print(standard_out.decode()) AttributeError: 'str' object has no attribute 'decode'
Comment 6 Matthieu Coudron 2016-02-16 07:04:18 EST
So I've started working on this,
https://github.com/teto/ns-3-dev-git/blob/test.py/test.py which can already launch several tests specified with successive "-s". I could not display the individual NS_TEST* to the console as I had envisioned since stdout/stderr are redirected so it would require some postprocess operations to display the file.

There is much code duplication to call test-runner --list. I suppose it should be called only once at the beginning.

As for waf, there is no workaround really, you just MUST run waf and test.py with the same interpreter.
Comment 7 Tom Henderson 2016-03-31 18:00:27 EDT
code review posted:
https://codereview.appspot.com/293030043/diff/1/test.py