Migrate launch tests to new launch_testing features & API #340
Conversation
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic
force-pushed
the
hidmic/import_launch_testing
branch
from
56e9ef9 to
f23ba1d
Compare
|
It might be worth declaring the tests with proc_info/proc_output as an argument. That works as well as referencing the magic local field self.proc_info/self.proc_output like: def test_@TEST_NUM_NODES@_nodes(self, proc_info):
launch_testing.asserts.assertExitCodes(proc_info)I added proc_info and proc_output to the test class before your suggestion to use arguments for the context returned from the test description (I like how you're using locals() in the test description, btw). It's something I was considering deprecating because passing as arguments feels so much cleaner. |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
hidmic
added
in review
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
| } | ||
| finally: | ||
| node.destroy_node() | ||
| TEST_CASES = { |
There was a problem hiding this comment.
Probably out of the scope of this PR, but IMHO this migrated test looks a little difficult to read. I mean, I needed to read the comments in launch/launch_testing/example_test_context.test.py to understand what was happening here (maybe, there is better documentation somewhere).
Passing test argument in the returned test context it's not quite intuitive. Also, it would be nice to have better parameterization support.
There was a problem hiding this comment.
I know it's hard to read, I did my best to not split it up while keeping some of the unittest like structure. Alternatively, we could partially recreate what pytest does with decorators to inject the test functions like it used to be but it seemed a bit too much for a migration.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Connected to ros2/launch#208.