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

👩‍🌾 Refactor UNIT_Server_TEST: move tests to integration #594

Merged
merged 4 commits into from
Feb 2, 2021

Conversation

j-rivero
Copy link
Contributor

Fixes #580

Move several server interactions and fuel cached to their own tests under integration, seems to me that they fit better there than in a "unit" test. I split the server Fixture not to copy/paste code all around.

@j-rivero j-rivero requested a review from chapulina as a code owner January 28, 2021 23:43
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Jan 28, 2021
@codecov
Copy link

codecov bot commented Jan 28, 2021

Codecov Report

Merging #594 (3571d38) into ign-gazebo4 (d8b9d0c) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           ign-gazebo4     #594   +/-   ##
============================================
  Coverage        77.36%   77.36%           
============================================
  Files              215      215           
  Lines            12029    12029           
============================================
  Hits              9306     9306           
  Misses            2723     2723           

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 d8b9d0c...c7f8691. Read the comment docs.

Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixes #580

It looks like it didn't 😢 I don't know what the TestSensorSystem has that causes this 😕

In any case, I think this refactoring is good. I'd just remove the duplicate test.

test/integration/multiple_servers.cc Outdated Show resolved Hide resolved
Move several server interactions and fuel cached to their own
tests in integration.

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
…into ign-gazebo4_refactor_server_test

Signed-off-by: Jose Luis Rivero <jrivero@osrfoundation.org>
@j-rivero j-rivero force-pushed the ign-gazebo4_refactor_server_test branch from c53a2a9 to d5a513c Compare January 29, 2021 11:09
@chapulina chapulina merged commit 701b6ee into ign-gazebo4 Feb 2, 2021
@chapulina chapulina deleted the ign-gazebo4_refactor_server_test branch February 2, 2021 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔮 dome Ignition Dome
Projects
None yet
Development

Successfully merging this pull request may close these issues.

👩‍🌾 UNIT_Server_TEST [ServerFixture.ServerConfigRealPlugin] flaky on GithubActions (timeout)
2 participants