Re-add truffleruby-head to CI

[eregon]

Same as jeremyevans/roda#200 but for Sequel.

I guess truffleruby was removed unintentionally in c11fbeb

[eregon]

A single failure:

  1) Error:
Blockless Ruby Filters#test_0076_should handle startless ranges:
ArgumentError: bad value for range
    (eval):1:in `test_0076_should handle startless ranges'
    <internal:core> core/kernel.rb:325:in `eval'
    /home/runner/work/sequel/sequel/spec/core/expression_filters_spec.rb:579:in `test_0076_should handle startless ranges'

That will most likely be fixed by oracle/truffleruby#2155
I'd suggest let's wait for that PR to be merged.

[jeremyevans]

Do you have an ETA for that truffleruby PR? Are you sure all other specs suites being run pass on truffleruby? Unlike Travis, GitHub Actions does not allow for marking that a specific matrix instance can fail, so we shouldn't include truffleruby until it can pass the entire suite.

[eregon]

I think it should be merged the coming week.
I'll push a commit to exclude that test, that way we know if the rest works fine.
I think continue-on-error can be used as a way to ignore a failing job, but it's rather unintuitive.

[jeremyevans]

I know little about GitHub Actions, but from what I've read, continue-on-error applies to the whole job, so it isn't something we could do for just truffleruby. We would have to use a separate yaml file for truffleruby in that case is my understanding. I'm not opposed to that approach if we need that.

[jeremyevans]

Looks like there is still issues beyond the core spec failure. So if you want to do this, you should probably add a separate yaml file for truffleruby that uses continue-on-error.

[eregon]

Looks like it's 2 failures, but maybe there are more in test suites running after.
continue-on-error would be fine, I'll try to get that to work.

I'll make a PR for the first 2 commits so we can already merge those separately: #1742

[eregon]

I tried continue-on-error, but then it looks like everything passed (which is a general issue of continue-on-error I'm afraid) and I'm not sure there is a reasonable way to know if it actually passed or not: https://github.com/eregon/sequel/runs/1476292451?check_suite_focus=true
And I'd like to be able to check with https://github.com/eregon/truffleruby-gem-tracker if it actually passed or failed.

Will try continue-on-error at the job level, to see if it changes anything.

If that doesn't work, probably the best would be to look at those failures and/or exclude them.

[jeremyevans]

You have to look at the spec output. There is a failure in the model spec, which means the plugin, SQLite, PostgreSQL, and MySQL specs didn't even run. continue-on-error only makes sense if you'll be manually monitoring the spec output.

[eregon]

With continue-on-error at the job level it looks a bit different, the job itself is shown as failed:
https://github.com/eregon/sequel/runs/1476368889?check_suite_focus=true
yet the entire workflow is marked as passing:
https://github.com/eregon/sequel/actions

I'll try that on this PR.

[eregon]

@jeremyevans While trying to debug this, I noticed something a bit odd.
Even on MRI, @o.associations just before

sequel/spec/model/associations_spec.rb

Line 4727 in 21ab4ce

@o.associations.fetch(:b).id.must_equal 1

is {:b=>#<Album::B @values={:id=>1, :x=>1}>}.
But as far as I could see, there is never a x column or attribute for Album::B, is that a bug?

Executing the failing spec examples individually on TruffleRuby actually passes, so it sounds like there is some global state that's not cleaned in that spec.

[eregon]

I've used the step-level continue-on-error due to several issues with job-level continue-on-error, and posted feedback to GitHub about it in actions/runner#2347.

I tried to investigate the failures but I couldn't figure out what was the issue, and the failing tests seem rather confusing to me so I'm not sure what's the expected behavior (notably is it OK to have one_to_one and a many_to_one association as reciprocal?).

So I suggest to merge this, now the CI should appear green, and I'll figure out a way to find out if the specs actually passed or not.

[jeremyevans]

I've used the step-level continue-on-error due to several issues with job-level continue-on-error, and posted feedback to GitHub about it in actions/runner#2347 (comment).

I think this works fine. Are you going to be monitoring the output of the truffleruby-head job until it passes?

I tried to investigate the failures but I couldn't figure out what was the issue, and the failing tests seem rather confusing to me so I'm not sure what's the expected behavior (notably is it OK to have one_to_one and a many_to_one association as reciprocal?).

Yes, that is OK and fairly normal. The spec works fine on both CRuby and JRuby, so seems unlikely to be a bug in the spec. The model spec database defaults to {:id=>1, :x=>1} (see spec/model/spec_helper.rb), that's were that value is coming from.

That model spec doesn't use before(:all), it creates a new class before spec execution and removes it after spec execution. There's definitely global state in the specs in general (the Sequel::Database object is the same), but I'm not sure that's the cause of the truffleruby failure.

So I suggest to merge this, now the CI should appear green, and I'll figure out a way to find out if the specs actually passed or not.

That seems reasonable. Thank you for working on this.

[eregon]

Are you going to be monitoring the output of the truffleruby-head job until it passes?

Yes, I'll keep an eye on it with https://github.com/eregon/truffleruby-gem-tracker
Right now it already says WARNING: jeremyevans/sequel was marked as failing but passed! which is a good reminder I need to improve the detection of step-level continue-on-error somehow.

To actually solve the failing specs it would be super helpful to have your help so I can understand better what it's testing.

[jeremyevans]

The spec you link to (line 4727) is testing that Sequel::Model#freeze will validate the object before freezing the associations hash. The associations hash starts out empty. The spec loads the association in the validate method, and fetch ensures that it is in the associations hash after calling freeze. However, that's not one of the two specs that's actually failing in https://github.com/jeremyevans/sequel/runs/1498946211.

One failing spec is:

    Album::B.dataset = Album::B.dataset.with_fetch(:album_id=>1, :id=>2)
    @o.b.must_equal Album::B.load(:id=>2, :album_id=>1)
    # TruffleRuby returns Album::B.load(:id=>1, :x=>1)

This shows that TruffleRuby is not picking up the overridden fetch results for Album::B.dataset, instead using the fetch results for the Database instance.

The other is:

    b = Album::B.load(:id=>2, :album_id=>nil)
    b.album = @o
    @o.associations[:b].must_be_nil

    @o = @o.dup
    b = Album::B.load(:id=>2, :album_id=>nil)
    b.album = @o
    @o.associations[:b].must_equal Album::B.load(:id=>2, :album_id=>1)
     # TruffleRuby returns nil instead of Album::B instance

This shows TruffleRuby is not setting the reciprocal association in @o when @o is the dup of the frozen object. It's expected that for frozen objects, that doesn't occur, and that part of the spec passes. Since the b association for Album is the reciprocal of the album association for B, when b.album=(@o) is called and @o is not frozen (since @o is a duped object), @o.associations[:b] should be the same as b.