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

ci: run the improved-sanity-test #899

Closed
wants to merge 2 commits into from
Closed

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Jul 27, 2017

Start running the improved-sanity-test from atomic-host-tests to make
sure the system works. This is an easy way to get PR-level comprehensive
integration tests for free.

But note that we don't mark it as required since the tests are not
stored here and it can happen that they need to be adjusted for new
rpm-ostree behaviours. In this way, this added check also allows us to
give a heads-up that breaking changes are coming.

I actually stopped using this a long time ago. Looking at it now, it
doesn't make much sense in multi-host situations. Let's just nix it.
Start running the improved-sanity-test from atomic-host-tests to make
sure the system works. This is an easy way to get PR-level comprehensive
integration tests for free.

But note that we *don't* mark it as required since the tests are not
stored here and it can happen that they need to be adjusted for new
rpm-ostree behaviours. In this way, this added check also allows us to
give a heads-up that breaking changes are coming.
@jlebon
Copy link
Member Author

jlebon commented Jul 27, 2017

Pending on projectatomic/atomic-host-tests#216.

@cgwalters
Copy link
Member

cgwalters commented Jul 27, 2017

Mmm...not opposed, but I still think the other way around would be more useful (a-h-t reusing existing tests), as that would have solved the problem we hit. a-h-t wouldn't have had to duplicate the status tests, and would transparently inherit any fixes we did, etc.

@jlebon
Copy link
Member Author

jlebon commented Jul 27, 2017

The value I think running the i-s-t at the PR level is that you get coverage for a lot of other things that your PR could affect. To me, what makes it valuable isn't so much the rpm-ostree specific tests (because we already have a pretty good testsuite), but all the other tests.

Actually, thinking more on this, maybe this makes more sense in ostree rather than rpm-ostree since the former has many more reverse deps and just higher potential for breaking platform changes.

@cgwalters
Copy link
Member

Where are we on this? Is it a WIP? Close?

@jlebon
Copy link
Member Author

jlebon commented Aug 24, 2017

bot, retest this please

@jlebon
Copy link
Member Author

jlebon commented Aug 24, 2017

I'd like to give it a chance and see how useful it is in practice. Then, we can reconsider down the road?

@jlebon
Copy link
Member Author

jlebon commented Aug 24, 2017

bot, retest this please

@cgwalters
Copy link
Member

@rh-atomic-bot r+ a35e47a

@rh-atomic-bot
Copy link

⌛ Testing commit a35e47a with merge f5bcd62...

rh-atomic-bot pushed a commit that referenced this pull request Aug 24, 2017
Start running the improved-sanity-test from atomic-host-tests to make
sure the system works. This is an easy way to get PR-level comprehensive
integration tests for free.

But note that we *don't* mark it as required since the tests are not
stored here and it can happen that they need to be adjusted for new
rpm-ostree behaviours. In this way, this added check also allows us to
give a heads-up that breaking changes are coming.

Closes: #899
Approved by: cgwalters
@rh-atomic-bot
Copy link

☀️ Test successful - status-atomicjenkins
Approved by: cgwalters
Pushing f5bcd62 to master...

@jlebon jlebon deleted the pr/aht branch October 3, 2017 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants