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 refactoring part 3 #464

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Conversation

mjcarroll
Copy link
Contributor

Continuation of #463

@mjcarroll mjcarroll requested a review from caguero as a code owner December 1, 2023 18:53
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Dec 1, 2023
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ca954cb) 87.23% compared to head (298b105) 85.64%.
Report is 2 commits behind head on gz-transport12.

❗ Current head 298b105 differs from pull request most recent head 5efc81c. Consider uploading reports for the commit 5efc81c to get more accurate results

Additional details and impacted files
@@                Coverage Diff                 @@
##           gz-transport12     #464      +/-   ##
==================================================
- Coverage           87.23%   85.64%   -1.60%     
==================================================
  Files                  60       60              
  Lines                5211     5211              
==================================================
- Hits                 4546     4463      -83     
- Misses                665      748      +83     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mjcarroll mjcarroll changed the base branch from gz-transport12 to mjcarroll/test_cleanup_part2 December 1, 2023 20:16
test/test_utils.hh Show resolved Hide resolved
Base automatically changed from mjcarroll/test_cleanup_part2 to gz-transport12 January 25, 2024 13:19
@mjcarroll mjcarroll force-pushed the mjcarroll/test_cleanup_part3 branch from b10196d to 70ee1a0 Compare January 25, 2024 15:57
@mjcarroll mjcarroll self-assigned this Jan 25, 2024
@azeey
Copy link
Contributor

azeey commented Jun 10, 2024

@caguero to take a look.

@caguero
Copy link
Collaborator

caguero commented Jun 28, 2024

@osrf-jenkins run tests please

@azeey azeey added the beta Targeting beta release of upcoming collection label Jul 29, 2024
Copy link
Collaborator

@caguero caguero left a comment

Choose a reason for hiding this comment

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

It looks good to me besides:

  • Some minor comments.
  • CI seems unhappy.


/// \brief Get the randomly generated partition for this test
/// \return string value of the partition
protected: [[nodiscard]] std::string Partition() const {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Opening bracket in the next line.

}

/// \brief Clean up the test fixture
protected: void TearDown() override {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Opening bracket in the next line.

std::make_unique<gz::utils::Subprocess>(_commandLine, environment);
}

private: std::string prevPartition;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Missing doxygen doc for these members.

Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
Signed-off-by: Michael Carroll <[email protected]>
@mjcarroll mjcarroll force-pushed the mjcarroll/test_cleanup_part3 branch from 5efc81c to d4966cc Compare August 5, 2024 18:23
@mjcarroll mjcarroll changed the base branch from gz-transport12 to main August 5, 2024 18:24
@mjcarroll
Copy link
Contributor Author

@caguero I'm going to just retarget this to main. It was targeted to garden currently which is almost eol.

@azeey azeey removed the beta Targeting beta release of upcoming collection label Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌱 garden Ignition Garden
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

4 participants