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.

Sign up for GitHub

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

Jump to bottom

Check merge bot and required pipelines #396

Closed
bkontur opened this issue Jul 23, 2024 · 8 comments · Fixed by #397
Closed

Check merge bot and required pipelines #396

bkontur opened this issue Jul 23, 2024 · 8 comments · Fixed by #397

Comments

@bkontur
Copy link
Contributor

bkontur commented Jul 23, 2024

E.g. this PR was merged but one pipeline integration-test (bridge-hub-kusama, bridge-hub-kusama-integration-tests) failed, how is that possible?

image
@bkontur
Copy link
Contributor Author

bkontur commented Jul 23, 2024

cc: @Bullrich

@Bullrich
Copy link
Contributor

Bullrich commented Jul 23, 2024 •
edited
Loading

The parameter continue-on-error: true seems to be the offender.

We should remove it in both places:

@bkontur bkontur mentioned this issue Jul 23, 2024
Merged
2 tasks
@bkontur
Copy link
Contributor Author

bkontur commented Jul 23, 2024

I removed continue-on-error: true here: #397 and I will also add a fix.

Maybe fail-fast: false looks better: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategyfail-fast

@bkontur
Copy link
Contributor Author

bkontur commented Jul 23, 2024

definitely, continue-on-error: true is not good, because tests failed and this pipeline passed :)
image

@bkontur
Copy link
Contributor Author

bkontur commented Jul 23, 2024

@Bullrich I removed continue-on-error: true and now that pipeline confirmTestPassed is marked as a skipped:
Test all features / All tests passed (pull_request) Skipped

image

So, is that confirmTestPassed marked as a required to block merge in the github configuration?

@Bullrich
Copy link
Contributor

It is working as intended, well done!

image

So, is that confirmTestPassed marked as a required to block merge in the github configuration?

Yes, we need to mark that test as the required one.

@bkchr
Copy link
Contributor

bkchr commented Jul 24, 2024

Marked all the "All ****" as required.

fellowship-merge-bot bot pushed a commit that referenced this issue Jul 24, 2024
@bkontur @Bullrich
Fix CI + one integration test for bridge-hub-kusama (#397)
c61e836 
Closes: #396

This PR:
- fixes CI test pipelines to block PRs when tests fail. See more details
[here](#396).
- Fixes an integration test for `bridge-hub-kusama-integration-tests`
that was not caught due to a previous CI bug.
- Includes some minor cleanup, changing `Versioned*::V4` to
`Versioned*::from` to use the latest version (for easier migration to
`xcm:v5`).

<!-- Remember that you can run `/merge` to enable auto-merge in the PR
-->

<!-- Remember to modify the changelog. If you don't need to modify it,
you can check the following box.
Instead, if you have already modified it, simply delete the following
line. -->

## TODO

- [x] fix the integration tests
- [X] Does not require a CHANGELOG entry

---------

Co-authored-by: Javier Bullrich <javier@bullrich.dev>
@bkontur
Copy link
Contributor Author

bkontur commented Jul 24, 2024

Fixed by #397

@bkontur bkontur closed this as completed Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix CI + one integration test for bridge-hub-kusama bkontur/runtimes
3 participants
@bkchr @bkontur @Bullrich