-
Notifications
You must be signed in to change notification settings - Fork 222
Fix #5546 - Adjust overrride of Dir.glob so that it finds json_schema attributes #5548
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.
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
Conversation
| # Without this 'path/**/*.rb' will match 'path/sub/file.rb' but not 'path/file.rb', cf #5546 | ||
| flags = flags | File::FNM_PATHNAME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't be fooled by the amount of code change, massive pain to diagnose and fix.
<broken record>We should drop the embedded shenanigans!</broken record>
🧪 Test Results DashboardSummary
|
| Run | XML File | Status |
|---|---|---|
| run1 | results.xml |
✅ Found |
| run3 | results.xml |
✅ Found |
| run2 | results.xml |
✅ Found |
… attributes
```
git@github.com:voxpupuli/json-schema.git
cd json-schema/lib
gvim test.rb
```
**test.rb**
```ruby
files = [
"json-schema/attributes/format.rb",
"json-schema/attributes/formats/custom.rb"
]
match_all_patterns = [
"json-schema/**/*.rb",
"json-schema/attributes/**/*.rb",
]
match_only_single_patterns = [
"json-schema/attributes/*.rb",
"json-schema/attributes/*/*.rb",
]
puts "| Pattern | File | Matches | Matches (FNM_PATHNAME) | Expected |"
puts "|--------------------------------|------------------------------------------|----------|------------------------|----------|"
match_all_patterns.each do |pattern|
files.each do |f|
matches = File.fnmatch(pattern, f)
matches_pathname = File.fnmatch(pattern, f, File::FNM_PATHNAME)
expected = Dir.glob(pattern).include?(f)
puts "| #{pattern.ljust(30)} | #{f.ljust(40)} | #{matches.to_s.center(8)} | #{matches_pathname.to_s.center(22)} | #{expected.to_s.center(8)} |"
end
end
match_only_single_patterns.each do |pattern|
files.each do |f|
matches = File.fnmatch(pattern, f)
matches_pathname = File.fnmatch(pattern, f, File::FNM_PATHNAME)
expected = Dir.glob(pattern).include?(f)
puts "| #{pattern.ljust(30)} | #{f.ljust(40)} | #{matches.to_s.center(8)} | #{matches_pathname.to_s.center(22)} | #{expected.to_s.center(8)} |"
end
end
```
```
$ruby test.rb
| Pattern | File | Matches | Matches (FNM_PATHNAME) | Expected |
|--------------------------------|------------------------------------------|----------|------------------------|----------|
| json-schema/**/*.rb | json-schema/attributes/format.rb | true | true | true |
| json-schema/**/*.rb | json-schema/attributes/formats/custom.rb | true | true | true |
| json-schema/attributes/**/*.rb | json-schema/attributes/format.rb | false | true | true |
| json-schema/attributes/**/*.rb | json-schema/attributes/formats/custom.rb | true | true | true |
| json-schema/attributes/*.rb | json-schema/attributes/format.rb | true | true | true |
| json-schema/attributes/*.rb | json-schema/attributes/formats/custom.rb | true | false | false |
| json-schema/attributes/*/*.rb | json-schema/attributes/format.rb | false | false | false |
| json-schema/attributes/*/*.rb | json-schema/attributes/formats/custom.rb | true | true | true |
```
d18db0d to
708b0a1
Compare
| def test_json_schema | ||
| require 'json-schema' | ||
|
|
||
| schema = { | ||
| "type" => "object", | ||
| "required" => ["a"], | ||
| "properties" => { | ||
| "a" => { | ||
| "type" => "integer" | ||
| } | ||
| } | ||
| } | ||
|
|
||
| data_valid = { "a" => 5 } | ||
| JSON::Validator.validate(schema, data_valid) | ||
|
|
||
| data_invalid = { "a" => "taco" } | ||
|
|
||
| error = assert_raises(JSON::Schema::ValidationError) do | ||
| # "The property '#/a' of type String did not match the following type: integer" | ||
| JSON::Validator.validate!(schema, data_invalid) | ||
| end | ||
| assert_match(/The property '#\/a' of type string did not match the following type: integer/, error.message) | ||
|
|
||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As @shorowit pointed out, a test is an excellent idea...
Before fix
1) Error:
EmbeddedRuby_Test#test_json_schema:
NoMethodError: undefined method `validate' for nil:NilClass
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/schema/validator.rb:25:in `block in validate'
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/schema/validator.rb:23:in `each'
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/schema/validator.rb:23:in `validate'
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/schema.rb:32:in `validate'
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/validator.rb:115:in `validate'
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/validator.rb:259:in `validate!'
:/ruby/3.2.0/gems/json-schema-4.3.1/lib/json-schema/validator.rb:243:in `validate'
/home/julien/Software/Others/OpenStudio/src/cli/test/test_embedded_ruby.rb:259:in `test_json_schema'
|
CI Results for 8f0f77f:
|
shorowit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirmed that the gem now runs.
Pull request overview
json-schemagem is broken #5546Demonstration
test.rb
Pull Request Author
src/model/test)src/energyplus/Test)src/osversion/VersionTranslator.cpp)Labels:
IDDChangeAPIChangePull Request - Ready for CIso that CI builds your PRReview Checklist
This will not be exhaustively relevant to every PR.