Skip to content

CI: Adjust error matching for newer json gem / newer JRuby #161

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

Merged
merged 1 commit into from
Aug 24, 2025
+15 −16
Merged

CI: Adjust error matching for newer json gem / newer JRuby #161

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ jobs:
- {os: ubuntu-24.04, ruby: '3.3'} # openssl 3
- {os: ubuntu-24.04, ruby: '3.4'} # openssl 3
- {os: ubuntu-24.04, ruby: 'jruby-9.4'}
- {os: ubuntu-24.04, ruby: 'jruby-9.4.8.0'}
- {os: ubuntu-24.04, ruby: 'jruby-9.4.13.0'}
- {os: windows-2025, ruby: '3.1'}
- {os: windows-2025, ruby: '3.2'} # openssl 3
- {os: windows-2025, ruby: '3.3'} # openssl 3
Expand Down
3 changes: 0 additions & 3 deletions spec/unit/file_system/path_pattern_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,6 @@
end

it 'globs wildcard patterns properly' do
# See PUP-11788 and https://github.com/jruby/jruby/issues/7836.
pending 'JRuby does not properly handle Dir.glob' if Puppet::Util::Platform.jruby?

dir = tmpdir('globtest')
create_file_in(dir, 'foo.pp')
create_file_in(dir, 'foo.pp.pp')
Expand Down
8 changes: 4 additions & 4 deletions spec/unit/functions/break_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ class does_break {
}
include(does_break)
CODE
end.to raise_error(/break\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/break\(\) from context where this is illegal .*/)
end

it 'does not provide early exit from a define' do
Expand All @@ -166,7 +166,7 @@ class does_break {
}
does_break { 'no_you_cannot': }
CODE
end.to raise_error(/break\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/break\(\) from context where this is illegal .*/)
end

it 'can be called when nested in a function to make that function behave as a break' do
Expand All @@ -191,7 +191,7 @@ class does_break {
$result = with(1) |$x| { with($x) |$x| {break() }}
notice $result
CODE
end.to raise_error(/break\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/break\(\) from context where this is illegal .*/)
end

it 'can not be called from top scope' do
Expand All @@ -201,7 +201,7 @@ class does_break {
# line 2
break()
CODE
end.to raise_error(/break\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/break\(\) from context where this is illegal .*/)
end
end
end
2 changes: 1 addition & 1 deletion spec/unit/functions/next_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,6 @@ class does_next {
# line 2
next()
CODE
end.to raise_error(/next\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/next\(\) from context where this is illegal .*/)
end
end
4 changes: 2 additions & 2 deletions spec/unit/functions/return_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class does_next {
$result = with(1) |$x| { with($x) |$x| {return(100) }}
notice $result
CODE
end.to raise_error(/return\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/return\(\) from context where this is illegal .*/)
end

it 'can not be called from top scope' do
Expand All @@ -104,6 +104,6 @@ class does_next {
# line 2
return()
CODE
end.to raise_error(/return\(\) from context where this is illegal \(file: unknown, line: 3\) on node.*/)
end.to raise_error(/return\(\) from context where this is illegal .*/)
end
end
2 changes: 1 addition & 1 deletion spec/unit/http/service/compiler_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -594,7 +594,7 @@
invalid_facts = Puppet::Node::Facts.new(certname, {'invalid_utf8_sequence' => "\xE2\x82".force_encoding('binary')})
expect {
subject.put_facts(certname, environment: 'production', facts: invalid_facts)
}.to raise_error(Puppet::HTTP::SerializationError, /Failed to serialize Puppet::Node::Facts to json: ("\\xE2" from ASCII-8BIT to UTF-8|partial character in source, but hit end)/)
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know yet what would have raised "\\xE2" from ASCII-8BIT to UTF-8. partial character in source, but hit end should come from the json gem java version: https://github.com/ruby/json/blob/master/java/src/json/ext/StringDecoder.java#L168-L176.

Maybe that exception just isn't raised anymore 🤔

}.to raise_error(Puppet::HTTP::SerializationError, /Failed to serialize Puppet::Node::Facts to json: /)
end
end

Expand Down
2 changes: 1 addition & 1 deletion spec/unit/parser/compiler_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ class #{name} {}
include #{name}
}
MANIFEST
}.to raise_error(Puppet::Error, /Class '#{name}' is already defined \(line: 1\); cannot be redefined as a node \(line: 2\) on node #{name}/)
}.to raise_error(Puppet::Error, /Class '#{name}' is already defined \(line: 1\); cannot be redefined as a node /)
end

it "evaluates the class if the node definition uses a regexp" do
Expand Down
2 changes: 1 addition & 1 deletion spec/unit/task_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@

expect {
tasks[0].metadata
}.to raise_error(Puppet::Module::Task::InvalidMetadata, /expected ':' after object key/)
}.to raise_error(Puppet::Module::Task::InvalidMetadata)
end

it 'returns empty hash for metadata when json metadata file is empty' do
Expand Down
6 changes: 3 additions & 3 deletions spec/unit/util/json_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
it 'raises an error if JSON is invalid' do
expect {
Puppet::Util::Json.load('{ invalid')
}.to raise_error(Puppet::Util::Json::ParseError, /expected object key, got 'invalid' at line 1 column 3/)
Copy link
Contributor

Choose a reason for hiding this comment

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

This also looks like an error from the json lib:

bastelfreak@bastelfreak-nb ~/code/json $ git grep 'expected object key'
ext/json/ext/parser/parser.c:                    raise_parse_error("expected object key, got %s", state);
ext/json/ext/parser/parser.c:                            raise_parse_error("expected object key, got: %s", state);
test/json/json_ext_parser_test.rb:      assert_equal "expected object key, got EOF at line 1 column 5", ex.message
bastelfreak@bastelfreak-nb ~/code/json $

Copy link
Contributor

@bastelfreak bastelfreak Jul 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hahaha. While digging around I found ruby/json#754, which I raised some months ago. From the developers:

So it's probably best if your test only assume a specific error class, but doesn't try to match the message content.

Still it would be nice to figure out it this passes with an older json gem version.

}.to raise_error(Puppet::Util::Json::ParseError)
end

it 'raises an error if the content is empty' do
Expand Down Expand Up @@ -74,7 +74,7 @@
it 'returns nil when the file is invalid JSON and debug logs about it' do
file_path = file_containing('input', '{ invalid')
expect(Puppet).to receive(:debug)
.with(/Could not retrieve JSON content .+: expected object key, got 'invalid' at line 1 column 3/).and_call_original
.with(/Could not retrieve JSON content/).and_call_original

expect(Puppet::Util::Json.load_file_if_valid(file_path)).to eql(nil)
end
Expand Down Expand Up @@ -102,7 +102,7 @@

expect {
Puppet::Util::Json.load_file(file_path)
}.to raise_error(Puppet::Util::Json::ParseError, /expected object key, got 'invalid' at line 1 column 3/)
}.to raise_error(Puppet::Util::Json::ParseError)
end

it 'raises an error when the filename is illegal' do
Expand Down