Skip to content

Conversation

etagwerker
Copy link
Collaborator

Hi there,

I'd like to mark the ruby-head job as "green" even if it fails. It's adding unnecessary noise when it's red.

Check list:

@etagwerker etagwerker force-pushed the fixes/jruby-ruby-head branch from ae51afa to 700a4dc Compare March 6, 2025 21:41
spec.add_development_dependency 'bundler', '>= 2.0.0'
if RUBY_PLATFORM == 'java'
spec.add_development_dependency 'pry-debugger-jruby'
spec.add_development_dependency 'jar-dependencies', '~> 0.4.1'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the comment about jar-dependencies in https://www.jruby.org/2025/02/11/jruby-9-4-12-0.html, "jar-dependencies upgraded to 0.5.4 to fix an issue parsing Maven output on Java versions 9 and higher," should we lock jar-dependencies to -> 0.5.4, or even remove the version restriction so as to pick up whatever jruby wants to install?

@faisal
Copy link
Contributor

faisal commented Mar 24, 2025

This seems worth taking.

@faisal
Copy link
Contributor

faisal commented Mar 27, 2025

I hope this will fix the failure we see in HEAD: cucumber/cucumber-ruby-core#292

@faisal
Copy link
Contributor

faisal commented Apr 7, 2025

@etagwerker Assuming you still want to make this change, how can we help?

@etagwerker etagwerker changed the title Fixes/jruby ruby head show green check mark even if ruby-head fails Apr 7, 2025
@etagwerker
Copy link
Collaborator Author

@faisal Yes, I'd still like to have this merged into main

I know it's a bit of OCD on my side, but I'd like the checks to be green for pull requests where only ruby-head fails.

Screenshot 2025-04-07 at 9 38 17 AM

I thought this change was going to be enough:

  Features:
    needs: pre_job # skip duplicates
    if: ${{ needs.pre_job.outputs.should_skip != 'true' }}
    runs-on: ubuntu-latest
    continue-on-error: ${{ matrix.experimental }}
    strategy:
      fail-fast: false
      matrix:
        ruby-version:
          - '3.1'
          - '3.2'
          - '3.3'
          - '3.4'
        experimental: [false]
        include:
          - ruby-version: 'ruby-head'
            experimental: true
            continue-on-error: true
          - ruby-version: 'jruby-9.4'
            experimental: true
    steps:
      - uses: actions/checkout@v2
      - name: Set up Ruby ${{ matrix.ruby-version }}
        uses: ruby/setup-ruby@v1
        with:
          ruby-version: ${{ matrix.ruby-version }}
          bundler-cache: true
          cache-version: 1
      - name: Run Cucumber
        run: bundle exec cucumber features --format progress --color

But it is still not doing what I want.

Any ideas?

@etagwerker etagwerker force-pushed the fixes/jruby-ruby-head branch from 81e528a to 78310f7 Compare April 7, 2025 13:40
@faisal
Copy link
Contributor

faisal commented Apr 7, 2025

[...]

But it is still not doing what I want.

Any ideas?

In cursory testing, I haven't been able to find a way to do this without changing runners. See actions/runner#2347.

@faisal
Copy link
Contributor

faisal commented Apr 8, 2025

It appears the check mark is always red if anything failed, even if the thing that failed is not required. Beyond OCD, I find this user-hostile because there's no quick way to know which PRs are ready for merging if any tasks aren't required to succeed.

As to how to address this. We could potentially have the experimental items always "succeed" but emit useful logging if they fail, then keep an eye on those logs in the experimental tasks.

Note that this is independent of what failures block a merge. If you have a branch rule that blocks merges you can specify which specific jobs must pass. We could move the experimental tasks out to "experimental" jobs and only require the "supported" jobs pass before merging. I think this factoring is worth doing in either case, and will try to take a swing at it when I get a moment.

@faisal
Copy link
Contributor

faisal commented Apr 8, 2025

Something like #517 sets up for having the branch rule block merges for the -supported jobs and MarkdownLint but not the -experimental jobs.

@etagwerker what do you think?

Also if you want to keep considering this problem then is it worth putting the gemspec changes into a focussed PR?

@faisal
Copy link
Contributor

faisal commented Apr 9, 2025

As an aside, https://github.com/faisal/rubycritic/tree/test_cucumber_should_work squashes the current Ruby 3.5 failure, at least. I think it's a bit 🤡 to check in, but it would get us back to green if that's the goal.

@JuanVqz
Copy link
Contributor

JuanVqz commented Oct 7, 2025

Hey @faisal, thanks for the effort you have been putting into this PR. if you continue on it, I can help you get it to the final line; just ping me.

Is this still valid, or is this the reason you opened the cucumber PR?

@faisal
Copy link
Contributor

faisal commented Oct 7, 2025

Hey @faisal, thanks for the effort you have been putting into this PR. if you continue on it, I can help you get it to the final line; just ping me.

Is this still valid, or is this the reason you opened the cucumber PR?

The cucumber PR was just to update to something more recent and hopefully less fragile.

I was working on this PR because (as @etagwerker points out) it would be nice to have failures in ruby-head (and perhaps other experimentally supported rubies) not show up as blocking failures. Cucumber example failures on ruby-head were a good case of this problem until I put in the workaround that #530 will undo, but now we see failures in jruby as well.

@faisal
Copy link
Contributor

faisal commented Oct 7, 2025

Hey @faisal, thanks for the effort you have been putting into this PR. if you continue on it, I can help you get it to the final line; just ping me.

Is this still valid, or is this the reason you opened the cucumber PR?

As to "still working on it" ... I think so, in the sense that I think we have an open issue on how to proceed here. What is the desired behavior if ruby-head or a jruby CI test fails but everything else passes? Should that permutation show green, or red?

@JuanVqz
Copy link
Contributor

JuanVqz commented Oct 9, 2025

Hey @faisal, thanks for the effort you have been putting into this PR. if you continue on it, I can help you get it to the final line; just ping me.

Is this still valid, or is this the reason you opened the cucumber PR?

As to "still working on it" ... I think so, in the sense that I think we have an open issue on how to proceed here. What is the desired behavior if ruby-head or a jruby CI test fails but everything else passes? Should that permutation show green, or red?

As for what I understood, it should show green status, have tou thought on a solution on how to make it possible?

@faisal
Copy link
Contributor

faisal commented Oct 14, 2025

As for what I understood, it should show green status, have tou thought on a solution on how to make it possible?

Take a look at #533

@faisal
Copy link
Contributor

faisal commented Oct 14, 2025

I believe the "green on experimental rubies" goal here will be reached once we land #534, and the improved JRuby bundling is handled as part of #514.

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

Successfully merging this pull request may close these issues.

3 participants