Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_generate_policy fails on Windows #143

Closed
jacobperron opened this issue Jul 8, 2019 · 5 comments · Fixed by #155
Closed

test_generate_policy fails on Windows #143

jacobperron opened this issue Jul 8, 2019 · 5 comments · Fixed by #155
Assignees
Labels
bug Something isn't working

Comments

@jacobperron
Copy link
Member

Bug report

Required Info:

  • Operating System:
    • Windows
  • Installation type:
    • source
  • Version or commit hash:
    • master

The test was introduced in #122.

It has been consistently failing the nightly Windows debug and release builds, e.g.

Output:

Error Message
AssertionError: assert 0 != 0  +  where 0 = <function main at 0x000002670D3A3C80>(argv=['security', 'generate_policy', 'C:\\WINDOWS\\TEMP\\tmpwsfvjedi\\test-policy.xml'])  +    where <function main at 0x000002670D3A3C80> = cli.main
Stacktrace
test\sros2\commands\security\verbs\test_generate_policy.py:67: in test_generate_policy_no_nodes
    assert cli.main(argv=[
E   AssertionError: assert 0 != 0
E    +  where 0 = <function main at 0x000002670D3A3C80>(argv=['security', 'generate_policy', 'C:\\WINDOWS\\TEMP\\tmpwsfvjedi\\test-policy.xml'])
E    +    where <function main at 0x000002670D3A3C80> = cli.main
@jacobperron jacobperron added the bug Something isn't working label Jul 8, 2019
@kyrofa
Copy link
Member

kyrofa commented Jul 8, 2019

The relevant line for the failure is here. I'm at a loss for why such a simple test-case would be failing on Windows and not Linux, and why only in the nightly and not otherwise. Any chance it runs tests in parallel in such a way that nodes ARE running when we don't expect any to be running?

@jacobperron
Copy link
Member Author

I've also seen the test failing on regular CI invocations (not just nightlies), though I'm not sure why the build passed on the original PR, maybe the test is flaky.

When I have a moment, I can try to repro locally.

@kyrofa
Copy link
Member

kyrofa commented Jul 8, 2019

Yeah, the only reason I can think for it to be flaky is if it happens to be picking up other nodes. Maybe we should be randomizing the DDS domain or something.

kyrofa added a commit to kyrofa/sros2 that referenced this issue Jul 10, 2019
The `test_generate_policy_no_nodes` test appears flaky under Windows. It
assumes the default DDS domain is empty of nodes, which may not always
be the case. Randomize the domain ID for that test.

Fix ros2#143

Signed-off-by: Kyle Fazzari <[email protected]>
@jacobperron
Copy link
Member Author

jacobperron commented Jul 20, 2019

I looked into this and it appears that nodes are being left over from a previous test before they ultimately time-out and are removed from the ROS graph. As a consequence, the sros2 generate policy tests sees one or more nodes that shouldn't be there.

I can reproduce by running tests for demo nodes prior to running the sros2 tests:

colcon test --packages-select demo_nodes_cpp && colcon test --packages-select sros2

@wjwwood speculated that it might be due to the way Windows is handling the SIGINT (or SIGTERM) sent by the launch testing framework. It's possible that the demo nodes do not exit cleanly and so they hang around for a while before timing out.

I tried waiting for the ROS graph to be empty before each test by checking that there are no nodes (except the one we're using for testing), but for some reason it always reported the graph as empty even though there were still topics/services lingering around. Next, I can try to update my implementation to wait for no topics, services, and actions before doing the test.

As an alternative, we could make the test simpler and assert that the generated policy contains the elements we expect as a subset, but I guess we'd have to disable the test for an empty ROS graph.

@ruffsl
Copy link
Member

ruffsl commented Jul 24, 2019

As an alternative, we could make the test simpler and assert that the generated policy contains the elements we expect as a subset, but I guess we'd have to disable the test for an empty ROS graph.

This sounds a bit simpler, and would be less likely to slow down the CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants