search

ros2 / sros2

tools to generate and distribute keys for SROS 2
Apache License 2.0
89 stars 44 forks source link

Wait for node discovery in test_generate_policy. #262

Closed hidmic closed 3 years ago

hidmic commented 3 years ago

Replacement for #260. Only wait when daemon is in use (if not, CLI will use a different node).

CI up to sros2:

codecov[bot] commented 3 years ago

Codecov Report

Merging #262 (75e4005) into master (ac0795c) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #262   +/-   ##
=======================================
  Coverage   77.25%   77.25%           
=======================================
  Files          25       25           
  Lines         664      664           
  Branches       55       55           
=======================================
  Hits          513      513           
  Misses        131      131           
  Partials       20       20
Flag Coverage Δ
unittests 77.25% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../hfyv6abjie8/sros2/sros2/sros2/command/security.py
ros_ws/src/hfyv6abjie8/sros2/sros2/sros2/errors.py
...hfyv6abjie8/sros2/sros2/sros2/keystore/__init__.py
...fyv6abjie8/sros2/sros2/sros2/keystore/_keystore.py
...v6abjie8/sros2/sros2/sros2/verb/generate_policy.py
...v6abjie8/sros2/sros2/sros2/keystore/_permission.py
...yv6abjie8/sros2/sros2/sros2/verb/create_enclave.py
...jie8/sros2/sros2/sros2/policy/defaults/__init__.py
...src/hfyv6abjie8/sros2/sros2/sros2/verb/__init__.py
...c/hfyv6abjie8/sros2/sros2/sros2/policy/__init__.py
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ac0795c...75e4005. Read the comment docs.

hidmic commented 3 years ago

@ros-pull-request-builder retest this please.

hidmic commented 3 years ago

Cool, thanks for the reviews! Does anybody have the bits to merge now? :sweat_smile:

clalancette commented 3 years ago

@SidFaber If you get a chance, can you review/merge this?

SidFaber commented 3 years ago

@kyrofa or @mikaelarguedas can you merge?

mikaelarguedas commented 3 years ago

How was it tested that this addresses the issue ? Can we confirm which test failure this is addressing ? which platform / architecture combination?

Looking at CI for this PR, it's not repeating any test so I'm not sure we can use that as reference to determine it reduces flakiness of a given test.

Comparing nightly CI referenced in the original issue: https://ci.ros2.org/view/nightly/job/nightly_osx_repeated/lastCompletedBuild/testReport/sros2.test.sros2.commands.security.verbs/test_generate_policy/test_generate_policy/ And the one after this was merged: https://ci.ros2.org/view/nightly/job/nightly_osx_repeated/2315/testReport/sros2.test.sros2.commands.security.verbs/test_generate_policy/test_generate_policy/

So maybe this PR did not have an effect ?