Make CI green by skipping failing ruby-head and jruby-head tests

collectiveidea / delayed_job

Database based asynchronous priority queue system -- Extracted from Shopify

http://groups.google.com/group/delayed_job

MIT License

4.81k stars 955 forks source link

Make CI green by skipping failing ruby-head and jruby-head tests #1162

Open johnnyshields opened 2 years ago

albus522 commented 2 years ago

What are you fixing?

johnnyshields commented 2 years ago

@albus522 the CI on master is not currently green. This PR aims to fix that.

It seems that JRuby 9.3 is the source of failure...

albus522 commented 2 years ago

It will green up when jruby supports Rails 7. It is acceptably red for now.

johnnyshields commented 2 years ago

@albus522 the issues aren't related to Rails 7.0.0, as you can see it's failing on Rails 6.1.0 too.

It's this bug: https://github.com/jruby/jruby/issues/6984

johnnyshields commented 2 years ago

I would suggest to merge this PR as it makes failures much more clear than what's on currently on DJ master. We should ignore failures on head etc. which master is not currently doing properly.

albus522 commented 2 years ago

This isn't fixing anything. It is just removing a bunch of stuff that is temporarily failing.

Moving the the continue-on-error into the steps is not an accurate move. If any of the steps fail, the next isn't going to work.

You can see the continue-on-error working here https://github.com/collectiveidea/delayed_job/actions/runs/411480881

It is not green right now because jruby does not register as >= 2.7.0 which is required by Active Support 7.

johnnyshields commented 2 years ago

@albus522 in the link you just sent, jruby-head and ruby-head are both appearing red (screenshot below). They should be green (i.e. skipped). -head tests are always very brittle and if we allow them to be red (non-skipped) then they be red for everyone raising a PR, i.e. the next time they break (which is almost guaranteed.)

I've updated the title of the PR to more accurately reflect the change here.

albus522 commented 2 years ago

I do not agree, failures should be red. The build finish step knows if the suite is considered passing. I agree that Github could make it more obvious and better distinguish allowed failures vs hard failures, but I will not be artificially marking everything green to get around a Github UI deficiency.

johnnyshields commented 2 years ago

Firstly, 100% agreed that Github should do a better job of marking skipped vs. failed (like Travis CI used to.) Unfortunately today Github does not.

IMHO CI should indicate whether the code is stable for all currently supported versions -- head / edge versions are not officially supported yet. Otherwise, the overall CI status is almost always going to show up as red, because one of Rails/Ruby/JRuby head / edge is pretty much always going to break somewhere.

albus522 commented 2 years ago

As I pointed out in the build above, the CI status is green. Some jobs are accurately red, but the overall build status is green

johnnyshields commented 2 years ago

Sure doesn't look that way to me on master

albus522 commented 2 years ago

Master isn't green right now.

And I see I botched the release commit message this round.