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

Polish tests and migrate or delete old tests #289

Merged
merged 12 commits into from Jun 16, 2018

Conversation

Projects
None yet
4 participants
@Martin-Idel-SI
Copy link

commented Jun 11, 2018

This PR does the following:

  • Polish test includes (include either gmock or gtest, but not both),
  • Use better methods to find properties to further increase maintainability of tests
  • Migrate a few old tests in the sense that all previously tested functionality is now tested by new automated (visual) tests
  • Delete all tests that are (now) superfluous as they are covered by unit or visual tests
  • Delete a bunch of old tests that seem to not test anything useful anymore. The last commit especially contains a few tests that we deem now irrelevant.

@rohbotics rohbotics added the in review label Jun 11, 2018

@Martin-Idel-SI

This comment has been minimized.

Copy link
Author

commented Jun 11, 2018

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood wjwwood added the 🔥 label Jun 12, 2018

@anhosi anhosi force-pushed the bsinno:feature/polish_and_port_tests branch from 8f1b150 to 89e7678 Jun 12, 2018

@anhosi

This comment has been minimized.

Copy link

commented Jun 12, 2018

new CI after rebase:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status
@anhosi

This comment has been minimized.

Copy link

commented Jun 12, 2018

Restarting mac build due to Jenkins NLP connection error:

  • macOS Build Status

@Martin-Idel-SI Martin-Idel-SI force-pushed the bsinno:feature/polish_and_port_tests branch from 89e7678 to 7fee156 Jun 13, 2018

@Martin-Idel-SI

This comment has been minimized.

Copy link
Author

commented Jun 13, 2018

CI after rebase:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Martin-Idel-SI added some commits May 22, 2018

Delete superfluous tests and files
- All tests have been superseded by unit or visual tests
Delete leftover test resources in rviz_common
The resources were used in a first stage of testing selection manager
Delete superfluous rendering test
- if this test fails, it doesn't really say anything about rviz anymore
- all visual tests would fail if this failed, so we would catch the error
- scene_graph introspection tests that are already there also give much more information on what went wrong
Polish includes in tests
- remove superfluous gtest include when gmock is already included
- adjust style of commits
Write test with wrong pointcloud data
- this was previously not caught by RViz, although a test was present to test against it
- delete this test (now automated)
Use findProperty instead of childAt where possible
- when adding properties, tests will not suddenly fail
- makes the test more explicit
Delete potentially superfluous tests
- The "connect_test" is actually a benchmark which documents something about how to use signals.
- The new_display_dialog_test seems to have hardly anything to do with rviz anymore
- The color_editor_test is no well documented test. Manual tests should be documented

@Martin-Idel-SI Martin-Idel-SI force-pushed the bsinno:feature/polish_and_port_tests branch from 7fee156 to 182992d Jun 15, 2018

@Martin-Idel-SI

This comment has been minimized.

Copy link
Author

commented Jun 15, 2018

CI after rebase:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status
    The current master contains an MSBuild warning for std::copy in map. This is not fixed by this PR.

@wjwwood wjwwood merged commit 95cfaf3 into ros2:ros2 Jun 16, 2018

@wjwwood wjwwood removed the in progress label Jun 16, 2018

@Martin-Idel-SI Martin-Idel-SI deleted the bsinno:feature/polish_and_port_tests branch Jun 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.