Shopify / yjit-bench

Set of benchmarks for the YJIT CRuby JIT compiler and other Ruby implementations.
MIT License
87 stars 22 forks source link

Allow TruffleRuby CI to fail without failing the whole job #333

Closed rwstauner closed 3 days ago

rwstauner commented 4 days ago

refs https://github.com/Shopify/yjit-bench/pull/332#issuecomment-2377505433

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

eregon commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

Let's do https://github.com/Shopify/yjit-bench/pull/334 then