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

Martin-Idel-SI
Copy link
Contributor

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 Waiting for review (Kanban column) label Jun 11, 2018
@Martin-Idel-SI
Copy link
Contributor Author

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 feature/polish_and_port_tests branch from 8f1b150 to 89e7678 Compare June 12, 2018 07:31
@anhosi
Copy link
Contributor

anhosi commented Jun 12, 2018

new CI after rebase:

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

@anhosi
Copy link
Contributor

anhosi commented Jun 12, 2018

Restarting mac build due to Jenkins NLP connection error:

  • macOS Build Status

@Martin-Idel-SI
Copy link
Contributor Author

CI after rebase:

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

@wjwwood wjwwood added in progress Actively being worked on (Kanban column) and removed 🔥 in review Waiting for review (Kanban column) labels Jun 14, 2018
@wjwwood wjwwood added in review Waiting for review (Kanban column) in progress Actively being worked on (Kanban column) and removed in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) labels Jun 15, 2018
- All tests have been superseded by unit or visual tests
The resources were used in a first stage of testing selection manager
- 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
- remove superfluous gtest include when gmock is already included
- adjust style of commits
- this was previously not caught by RViz, although a test was present to test against it
- delete this test (now automated)
- when adding properties, tests will not suddenly fail
- makes the test more explicit
- 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
Copy link
Contributor Author

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 Actively being worked on (Kanban column) label Jun 16, 2018
@Martin-Idel-SI Martin-Idel-SI deleted the feature/polish_and_port_tests branch June 18, 2018 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants