Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[wip] Rework spec for different JRuby implementation. #611

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

[wip] Rework spec for different JRuby implementation. #611

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
511b2b8
snip
JonRowe Aug 19, 2024
08d2bcd
more snip
JonRowe Sep 6, 2024
cedf81e
WIP fix for Jruby spec
JonRowe Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 1 addition & 61 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,33 +42,13 @@ jobs:
matrix:
ruby:
- '3.3'
- '3.2'
- '3.1'
- '3.0'
- 2.7
- 2.6
- 2.5
- 2.4
- 2.3
- 2.2
env:
-
DIFF_LCS_VERSION: "> 1.4.3"
include:
- ruby: ruby-head
env:
RUBY_HEAD: true
- ruby: jruby-9.2.13.0
env:
JRUBY_OPTS: "--dev"
- ruby: 2.7
name_extra: "with diff-lcs 1.3"
env:
DIFF_LCS_VERSION: "~> 1.3.0"
- ruby: 2.7
name_extra: "with diff-lcs 1.4.3"
env:
DIFF_LCS_VERSION: "1.4.3"
fail-fast: false
continue-on-error: ${{ matrix.allow_failure || endsWith(matrix.ruby, 'head') }}
env: ${{ matrix.env }}
Expand Down Expand Up @@ -101,22 +81,6 @@ jobs:
fail-fast: false
matrix:
container:
- version: "2.1.9"
tag: ghcr.io/rspec/docker-ci:2.1.9
post: git config --global --add safe.directory `pwd`
- version: "2.0"
tag: ghcr.io/rspec/docker-ci:2.0.0
- version: "1.9.3"
tag: ghcr.io/rspec/docker-ci:1.9.3
- version: "1.9.2"
tag: ghcr.io/rspec/docker-ci:1.9.2
options: "--add-host rubygems.org:151.101.129.227 --add-host api.rubygems.org:151.101.129.227"
- version: "1.8.7"
tag: ghcr.io/rspec/docker-ci:1.8.7
options: "--add-host rubygems.org:151.101.129.227 --add-host api.rubygems.org:151.101.129.227"
- version: "REE"
tag: ghcr.io/rspec/docker-ci:ree
options: "--add-host rubygems.org:151.101.129.227 --add-host api.rubygems.org:151.101.129.227"
- version: "JRuby 1.7"
tag: ghcr.io/rspec/docker-ci:jruby-1.7
- version: "JRuby 1.7 1.8 mode"
Expand All @@ -137,28 +101,4 @@ jobs:
- run: ${{ matrix.container.pre }}
- run: script/legacy_setup.sh
- run: ${{ matrix.container.post }}
- run: bundle exec bin/rspec
- run: bundle exec script/cucumber.sh

windows:
name: Ruby ${{ matrix.ruby }} (Windows)
runs-on: windows-latest
strategy:
matrix:
ruby:
- 2.7
- 2.6
- 2.5
- 2.4
- 2.3
- 2.2
fail-fast: false
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
bundler: '2.2.22'
ruby-version: ${{ matrix.ruby }}
bundler-cache: true
- run: choco install ansicon
- run: bundle exec rspec --backtrace
- run: bundle exec bin/rspec spec/rspec/support/spec/stderr_splitter_spec.rb:103
20 changes: 1 addition & 19 deletions script/run_build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,4 @@ fi

fold "binstub check" check_binstubs

fold "specs" run_specs_and_record_done

if additional_specs_available; then
fold "additional specs" run_additional_specs
fi

fold "cukes" run_cukes

if documentation_enforced; then
fold "doc check" check_documentation_coverage
fi

if supports_cross_build_checks; then
fold "one-by-one specs" run_specs_one_by_one
export NO_COVERAGE=true
run_all_spec_suites
else
echo "Skipping the rest of the build on non-MRI rubies"
fi
bundle exec rspec spec/rspec/support/spec/stderr_splitter_spec.rb:103
28 changes: 18 additions & 10 deletions spec/rspec/support/spec/stderr_splitter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,26 +101,34 @@

# This spec replicates what matchers do when capturing stderr, e.g `to_stderr_from_any_process`
it 'is able to restore the stream from a cloned StdErrSplitter' do
if RSpec::Support::Ruby.jruby?
skip """
This spec is currently unsupported on JRuby on CI due to tempfiles not being
a file, this situtation was discussed here https://github.com/rspec/rspec-support/pull/598#issuecomment-2200779633
"""
end

cloned = splitter.clone
expect(splitter.to_io).not_to be_a(File)
expect(splitter.to_io).to be_stderr

tempfile = Tempfile.new("foo")
begin
splitter.reopen(tempfile)
expect(splitter.to_io).to be_a(File)
expect(splitter.to_io).to_not be_stderr
Copy link
Member Author

Choose a reason for hiding this comment

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

@nevinera Unless I'm mistaken this represents the same effect as what you were testing, I'm not sure if I will merge this yet but it at least works for some JRuby

Copy link
Contributor

@nevinera nevinera Aug 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Ah, I think the part I didn't follow was the bottom method - I assumed be_stderr was calling a stderr? method I wasn't aware of on the stream/splitter >.<

I'm not confident about how consistent that inspect is, but I'll trust your plan :-)

(Is that trick sturdy enough to use in here?)

ensure
splitter.reopen(cloned)
tempfile.close
tempfile.unlink
end
# This is the important part of the test that would fail without proper cloning hygeine
expect(splitter.to_io).not_to be_a(File)
expect(splitter.to_io).to be_stderr
end

# Detecting STDERR in a way that doesn't use a reference
if STDERR.respond_to?(:path)
def be_stderr
have_attributes({:path => '<STDERR>'})
end
elsif STDERR.inspect =~ /STDERR/
def be_stderr
have_attributes({:inspect => "#<IO:<STDERR>>"})
end
else
def be_stderr
raise skip "JRuby < 9.0.0.0 doesn't have a predictable identifier for stdout"
end
end
end
Loading