delayed_job icon indicating copy to clipboard operation
delayed_job copied to clipboard

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

Open johnnyshields opened this issue 3 years ago • 12 comments

johnnyshields avatar Jan 18 '22 07:01 johnnyshields

What are you fixing?

albus522 avatar Jan 18 '22 15:01 albus522

@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...

johnnyshields avatar Jan 18 '22 17:01 johnnyshields

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

albus522 avatar Jan 18 '22 17:01 albus522

@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 avatar Jan 18 '22 17:01 johnnyshields

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.

johnnyshields avatar Jan 18 '22 18:01 johnnyshields

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.

albus522 avatar Jan 18 '22 18:01 albus522

@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.

image

johnnyshields avatar Jan 18 '22 18:01 johnnyshields

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.

albus522 avatar Jan 18 '22 18:01 albus522

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.

johnnyshields avatar Jan 18 '22 18:01 johnnyshields

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 Screen Shot 2022-01-18 at 1 50 48 PM .

albus522 avatar Jan 18 '22 18:01 albus522

Sure doesn't look that way to me on master

image

johnnyshields avatar Jan 18 '22 20:01 johnnyshields

Master isn't green right now.

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

albus522 avatar Jan 18 '22 22:01 albus522