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

RootFS: Upgrade testsuite #449

Merged
merged 3 commits into from
Jan 29, 2020
Merged

RootFS: Upgrade testsuite #449

merged 3 commits into from
Jan 29, 2020

Conversation

staticfloat
Copy link
Member

This adds a few small improvements to our testsuite:

@staticfloat staticfloat requested a review from giordano January 29, 2020 23:16
Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

This looks good to me!

There is one thing that was down in my TODO list that however can be done here since we're upgrading the testsuite: can we move test/build_tests/cxxstringabi_tests/lib.cc from BinaryBuilder to here so that C++ string ABI checking is actually exercised when running the Shard testsuite? Code coverage shows that C++ string ABI detection is completely untested.

@staticfloat
Copy link
Member Author

That should already be exercised by the cxx/dyn_link test suite. If coverage is showing it as not being run, something else is going wrong.

@staticfloat
Copy link
Member Author

The link you posted shows that detect_cxxstring_abi seems to be getting run (although we're never going down the "bad" paths; I think we need to run the test with a purposefully mismatched CompilerABI)

Copy link
Member

@giordano giordano left a comment

Choose a reason for hiding this comment

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

Ok, yes, we need better tests in BinaryBuilder, but the testsuite here should be fine

@staticfloat staticfloat merged commit 7f16e0b into master Jan 29, 2020
@staticfloat staticfloat deleted the sf/testsuite_bestsuite branch January 29, 2020 23:55
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.

2 participants