Skip to content

Conversation

@rwstauner
Copy link
Contributor

@rwstauner rwstauner commented Sep 26, 2024

refs #332 (comment)

While this appears to work, it may make it hard to notice when the TruffleRuby build is failing (if the individual test looks green):
image

@rwstauner rwstauner requested a review from k0kubun September 26, 2024 19:13
@eregon
Copy link
Member

eregon commented Sep 26, 2024

Switching to a TruffleRuby release seems better, because indeed this is completely hiding the fact the job fail and e.g. a breaking change in the harness would be unnoticed.
(https://github.com/eregon/truffleruby-gem-tracker would still notice the failure, but not PRs on this repo)

Alternatively, IMO it's also fine to keep things as-is, the job only started to fail when updating protoboeuf in #325.
Skipping the benchmark and pinging me sounds a reasonable workflow to me, that doesn't seem to be much overhead for yjit-bench maintainers, is it?

@eregon
Copy link
Member

eregon commented Sep 26, 2024

To get some data I looked at failed runs at https://github.com/Shopify/yjit-bench/actions/workflows/test.yml?query=branch%3Amain
In the first 5 pages, all red jobs seem to be when ruby-head was failing, not truffleruby-head, except https://github.com/Shopify/yjit-bench/actions/runs/11002156114/job/30548468755 where they were both failing for the same reason.
IOW ruby-head seems more unstable than truffleruby-head recently, so I think there is no need to change anything here.

@k0kubun
Copy link
Member

k0kubun commented Sep 26, 2024

that doesn't seem to be much overhead for yjit-bench maintainers, is it?

To me, it does. Nothing feels like more of an overhead than false positives for the repository.

Switching to a TruffleRuby release

I'm open to trying this first too. As long as the job doesn't fail unless we bump the TruffleRuby version or we push CRuby-specific code, I'm good.

@eregon
Copy link
Member

eregon commented Sep 27, 2024

Let's do #334 then

@maximecb maximecb closed this Sep 27, 2024
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.

5 participants