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

Get gtest 1.10.0 and parametrize tests #75

Merged
merged 8 commits into from
Mar 3, 2021
Merged

Conversation

antalszava
Copy link
Contributor

@antalszava antalszava commented Mar 2, 2021

Changes

  • Changes to CI to get the latest release of Google test available on GitHub (v1.10.0), see release notes
  • Creates parametrized tests

@codecov
Copy link

codecov bot commented Mar 2, 2021

Codecov Report

Merging #75 (91b7e54) into master (5b48b7f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   62.80%   62.80%           
=======================================
  Files           5        5           
  Lines         121      121           
=======================================
  Hits           76       76           
  Misses         45       45           

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 5b48b7f...4afe3e9. Read the comment docs.

@antalszava antalszava changed the title Parametrize tests Get gtest 1.10.0 and parametrize tests Mar 2, 2021
Comment on lines +25 to +26
int input = std::get<0>(GetParam());
int expected = std::get<1>(GetParam());
Copy link

@chaserileyroberts chaserileyroberts Mar 2, 2021

Choose a reason for hiding this comment

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

std::get<0>(GetParam()): is the C++ equivalent of python's a[0] lol

@antalszava antalszava marked this pull request as ready for review March 2, 2021 22:40
Copy link
Contributor

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @antalszava, looks great!

I'm still not convinced of the benefits of this approach versus the "good old for loop", since for me there is also a cost to this approach of more complexity and less understandable code.

But maybe where this approach shines is in more advanced test cases. I'm good to go with this approach for now.

}
TEST_P(Exp2TestFixture, CheckExp2Results) {
int input = std::get<0>(GetParam());
int expected = std::get<1>(GetParam());
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you get any warnings when using an int type? For me, it was giving a warning of comparing int and size_t.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, same here, warnings are emitted there.

@antalszava
Copy link
Contributor Author

@trbromley agreed that a for loop is actually briefer in these cases! One advantage with parameterized test cases is that they can be easier to parse on test failure as the cases are separated based on parameters. Though in the cases of this PR, I agree that there is not such a big difference.

@antalszava antalszava merged commit 5c869c3 into master Mar 3, 2021
@antalszava antalszava deleted the parametrize_tests branch March 3, 2021 19:03
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

3 participants