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

Remove the red workflows from CI #147

Open
galargh opened this issue Aug 21, 2023 · 2 comments
Open

Remove the red workflows from CI #147

galargh opened this issue Aug 21, 2023 · 2 comments

Comments

@galargh
Copy link
Contributor

galargh commented Aug 21, 2023

We're running a lot of different workflows on PRs to this repo now. I find it extremely confusing to have red runs on PRs. Also, I think it promotes the expectation that it's OK for CI on PRs to be red.

If we expect and want to keep expecting a failure of some workflow, I think we should catch such failure in the workflow and report workflow success on that failure, and workflow failure otherwise. This would send a clear signal to the contributors: 🟢 - the workflow reached an expected result (be it success or failure), 🔴 - the workflow reached an unexpected result (you have to look into it).

cc @laurentsenta

@laurentsenta
Copy link
Contributor

laurentsenta commented Aug 21, 2023

Agreed, thanks for opening this!

These should solve themselves:

"Test Kubo (e2e)" deserves some work, a proposal:
We could tweak the workflow so it's green when the conformance runs as expected. No matter the result.
We could also print the job summary test as a PR comment because we won't check the job summary if it's green.
This way, a maintainer would see the failure even if the job is green.

This way we can strike a balance between "I changed a predicate, Kubo doesn't pass half of the test suite now, let's not be silent about it" and "Kubo is missing a patch, the test suite is failing as expected, let's not output RED status in CI" like this case #131 (comment)

This test will fail against both Kubo master and Kubo latest until ipfs/kubo#10024 is merged.

@laurentsenta
Copy link
Contributor

laurentsenta commented Oct 2, 2023

We worked around most of the red CI, but before we close this,
we'd need to render a delta in the test results ("-200 tests, + 5 failures") to detect errors like panics or failing test case, see #172

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

No branches or pull requests

2 participants