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

Run cmake tests in CI #915

Merged
merged 2 commits into from
Aug 12, 2021
Merged

Conversation

Joshua-Anderson
Copy link
Contributor

Originating Project/Creator
Affected Component
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n)
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

Run the cmake test cases as a github actions parallel job.

Move install test into basic test to test both cases with only a single build
Remove UT test because it's tested elsewhere as part of the build process and takes a significant amount of time
@Joshua-Anderson
Copy link
Contributor Author

Joshua-Anderson commented Aug 12, 2021

It looks like the cmake tests were broken so I cleaned them up a bit:

  • Corrected dependencies so that dynamic libraries work again
  • Combined basic and install test so that we only have to do one cmake build to test both cases, shaves ~2 min off runtime
  • Removed unit test cmake test... this was running the full build and full framework test and was quite slow. It wasn't really checking anything other than that the unit test target worked. Since we already run the unit tests so many times I was thinking that this wasn't providing enough value to justify adding 5 minutes of CI time. What are your thoughts @LeStarch?

@LeStarch
Copy link
Collaborator

@Joshua-Anderson given we are essentially rewriting the CMake system, I am hesitant to enable this now. I don't see the benefit of this given it will likely only trap the v3.0.0 stuff and need to be reworked at that time.

Thoughts?

Since it is working and fairly fast, I think it would be reasonable to merge, but I can tell you it is already broken with the v3.0.0 items, and these tests will need to change not the other way around.

@LeStarch
Copy link
Collaborator

Rethought, the cmake should have tests, and those tests should pass. All this does is make sure the tests are delivered.

@LeStarch LeStarch merged commit 43767bd into nasa:devel Aug 12, 2021
@Joshua-Anderson Joshua-Anderson deleted the ci-test-cmake-tests branch August 12, 2021 21:29
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