Skip to content

Conversation

@ima1zumi
Copy link
Member

The system on CI rake version is 13.0.6, while the bundle installing rake version is 13.1.0.
IRB's test on CI depends on the system's rake document.
Match the rake version installed on system to the version installed by bundler.

The system on CI rake version is 13.0.6, while the bundler's rake version is 13.1.0.
IRB's test depends on the system's rdoc version of rake.
Match the rake version installed by bundler to the version installed on the system.
@tompng tompng merged commit dfa527a into ruby:master Oct 30, 2023
@st0012
Copy link
Member

st0012 commented Oct 30, 2023

Thank you for the fix but I don't fully understand the root cause and how this resolved it. Is it because 13.1.0 is installed without document so IRB couldn't find the doc? Do we rely on rake's document, or we rely on it to generate document?

@tompng
Copy link
Member

tompng commented Oct 30, 2023

The failing test test_autocomplete_with_showdoc_in_gaps_on_narrow_screen_right and test_autocomplete_with_showdoc_in_gaps_on_narrow_screen_left is showing document of String.
It is depending on rake because rake includes document of string and system doc is not available on ci.

$ mv /opt/ruby/share/ri/3.3.0+0/system /opt/ruby/share/ri/3.3.0+0/system_
$ ri String
= String < Object

(from gem rake-13.1.0)
------------------------------------------------------------------------
= Instance methods:

  ext, pathmap, pathmap_explode, pathmap_partial, pathmap_replace

@st0012
Copy link
Member

st0012 commented Oct 30, 2023

Wow that's pretty wild. Can we maybe use StandardError in the test case to make sure we always look for the doc from Ruby Core?

@tompng
Copy link
Member

tompng commented Oct 30, 2023

Ruby core's document including StandardError is not available on ci. (I don't. know why)

I don't think we should rely on it. If we rely on it, maybe we should explicitly show Rake's document in test.

write("require 'rake')
write("Rake\C-i")

There are some workaround variation. I merged this because I think it's worth first add this workaround to make ci green.

@st0012
Copy link
Member

st0012 commented Oct 30, 2023

I merged this because I think it's worth first add this workaround to make ci green.

Oh I totally agree with this 👍
I'm asking to make sure we know what to do next so we don't get bitten by the same problem again later.

I think we may just use IRB's own content for this. I will open a PR later.

@ima1zumi ima1zumi deleted the workaround-for-ci branch October 30, 2023 16:55
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