metanorma / pubid-ieee

PubID spec and implementation for IEEE deliverables
BSD 2-Clause "Simplified" License
1 stars 0 forks source link

fix rspec for months #86

Closed opoudjis closed 11 months ago

opoudjis commented 1 year ago

Metanorma PR checklist

opoudjis commented 1 year ago

And please release gem when you've approved this, I want to resume work on https://github.com/metanorma/metanorma-ieee/issues/112

mico commented 1 year ago

@opoudjis what the point of this change?

opoudjis commented 1 year ago

Not to crash rspec. Run it and see: you are assuming months are integers, but without this change, they are strings

Στις Κυρ, 14 Μαΐ 2023, 15:01 ο χρήστης Artur Komarov < @.***> έγραψε:

@opoudjis https://github.com/opoudjis what the point of this change?

— Reply to this email directly, view it on GitHub https://github.com/metanorma/pubid-ieee/pull/86#issuecomment-1546883283, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAOSR7YSTEYJLO2JUKSRYBTXGDCP3ANCNFSM6AAAAAAX5FNIMA . You are receiving this because you were mentioned.Message ID: @.***>

ronaldtse commented 1 year ago

This change needs to be rebased on top of new main.

mico commented 1 year ago

Not to crash rspec. Run it and see: you are assuming months are integers, but without this change, they are strings Στις Κυρ, 14 Μαΐ 2023, 15:01 ο χρήστης Artur Komarov < @.***> έγραψε:

Could you show me rspec that is crashing? sprintf('%02d' already doing conversion to integer.

opoudjis commented 1 year ago
Failures:

  1) Pubid::Ieee::Identifier parse specific identifiers IEC/IEEE 60076-16 Edition 2.0 2018-09 behaves like converts pubid to pubid converts pubid to pubid
     Failure/Error: result += "-#{sprintf('%02d', month)}"

     ArgumentError:
       invalid value for Integer(): "09"
     Shared Example Group: "converts pubid to pubid" called from ./spec/pubid_ieee/identifiers_parsing_spec.rb:111
     # ./lib/pubid/ieee/identifier/base.rb:203:in `sprintf'
     # ./lib/pubid/ieee/identifier/base.rb:203:in `edition'
     # ./lib/pubid/ieee/identifier/base.rb:133:in `parameters'
     # ./lib/pubid/ieee/identifier/base.rb:120:in `dual_identifier'
     # ./lib/pubid/ieee/identifier/base.rb:105:in `to_s'
     # ./spec/pubid_ieee/identifiers_parsing_spec.rb:17:in `block (3 levels) in <module:Ieee>'

  2) Pubid::Ieee::Identifier parse specific identifiers IEC 61523-3 First edition 2004-09 behaves like converts pubid to pubid converts pubid to pubid
     Failure/Error: result += "-#{sprintf('%02d', month)}"

     ArgumentError:
       invalid value for Integer(): "09"
     Shared Example Group: "converts pubid to pubid" called from ./spec/pubid_ieee/identifiers_parsing_spec.rb:146
     # ./lib/pubid/ieee/identifier/base.rb:203:in `sprintf'
     # ./lib/pubid/ieee/identifier/base.rb:203:in `edition'
     # ./lib/pubid/ieee/identifier/base.rb:133:in `parameters'
     # ./lib/pubid/ieee/identifier/base.rb:120:in `dual_identifier'
     # ./lib/pubid/ieee/identifier/base.rb:105:in `to_s'
     # ./spec/pubid_ieee/identifiers_parsing_spec.rb:17:in `block (3 levels) in <module:Ieee>'

  3) Pubid::Ieee::Identifier parse specific identifiers IEC/IEEE 62582-1:2011 Edition 1.0 2011-08 behaves like converts pubid to pubid converts pubid to pubid
     Failure/Error: result += "-#{sprintf('%02d', month)}"

     ArgumentError:
       invalid value for Integer(): "08"
     Shared Example Group: "converts pubid to pubid" called from ./spec/pubid_ieee/identifiers_parsing_spec.rb:697
     # ./lib/pubid/ieee/identifier/base.rb:203:in `sprintf'
     # ./lib/pubid/ieee/identifier/base.rb:203:in `edition'
     # ./lib/pubid/ieee/identifier/base.rb:133:in `parameters'
     # ./lib/pubid/ieee/identifier/base.rb:120:in `dual_identifier'
     # ./lib/pubid/ieee/identifier/base.rb:105:in `to_s'
     # ./spec/pubid_ieee/identifiers_parsing_spec.rb:17:in `block (3 levels) in <module:Ieee>'

Finished in 20.56 seconds (files took 0.372 seconds to load)
321 examples, 3 failures

Failed examples:

rspec ./spec/pubid_ieee/identifiers_parsing_spec.rb[1:1:13:1:1] # Pubid::Ieee::Identifier parse specific identifiers IEC/IEEE 60076-16 Edition 2.0 2018-09 behaves like converts pubid to pubid converts pubid to pubid
rspec ./spec/pubid_ieee/identifiers_parsing_spec.rb[1:1:18:1:1] # Pubid::Ieee::Identifier parse specific identifiers IEC 61523-3 First edition 2004-09 behaves like converts pubid to pubid converts pubid to pubid
rspec ./spec/pubid_ieee/identifiers_parsing_spec.rb[1:1:96:1:1] # Pubid::Ieee::Identifier parse specific identifiers IEC/IEEE 62582-1:2011 Edition 1.0 2011-08 behaves like converts pubid to pubid converts pubid to pubid

When I run it, month is NOT an integer, it is a string. Passing it to "%d" is an error.

mico commented 1 year ago

@opoudjis I didn't manage to reproduce the error, even though I understand the issue:

3.1.1 :006 > sprintf('%02d', "02")
 => "02"
3.1.1 :007 > sprintf('%02d', "08")
(irb):7:in `sprintf': invalid value for Integer(): "08" (ArgumentError)

But while debugging the test:

0> month
=> "09"@35

0> month.class
=> Parslet::Slice

0> month.to_i
=> 9

0> Integer(month)
=> 9

0> "-#{sprintf('%02d', month)}"
=> "-09"

So the month is not a String, but Parslet::Slice and Parslet::Slice#to_i returns correct value. We can just merge your pull request, but I would like to find where the issue comes from, because we can face it in other pubid-* gems.

I tried different ruby versions (2.5, 2.7, 3.1), but it all passes the test.

The only case I can imagine, it might be related to parslet version you use. So I added pull request with locked parslet version: https://github.com/metanorma/pubid-ieee/pull/93 Could you check how does it work for you?

opoudjis commented 1 year ago

Even with parslet at 2.0.0 and main branch, I am still getting this issue:

  1) Pubid::Ieee::Identifier parse specific identifiers IEC/IEEE 60076-16 Edition 2.0 2018-09 behaves like converts pubid to pubid converts pubid to pubid
     Failure/Error: result += "-#{sprintf('%02d', month)}"

     ArgumentError:
       invalid value for Integer(): "09"
     Shared Example Group: "converts pubid to pubid" called from ./spec/pubid_ieee/identifiers_parsing_spec.rb:111
     # ./lib/pubid/ieee/identifier/base.rb:231:in `sprintf'
     # ./lib/pubid/ieee/identifier/base.rb:231:in `edition'
     # ./lib/pubid/ieee/identifier/base.rb:156:in `parameters'
     # ./lib/pubid/ieee/identifier/base.rb:112:in `to_s'
     # ./spec/pubid_ieee/identifiers_parsing_spec.rb:17:in `block (3 levels) in <module:Ieee>'

  2) Pubid::Ieee::Identifier parse specific identifiers IEC 61523-3 First edition 2004-09 behaves like converts pubid to pubid converts pubid to pubid
     Failure/Error: result += "-#{sprintf('%02d', month)}"

     ArgumentError:
       invalid value for Integer(): "09"
     Shared Example Group: "converts pubid to pubid" called from ./spec/pubid_ieee/identifiers_parsing_spec.rb:146
     # ./lib/pubid/ieee/identifier/base.rb:231:in `sprintf'
     # ./lib/pubid/ieee/identifier/base.rb:231:in `edition'
     # ./lib/pubid/ieee/identifier/base.rb:156:in `parameters'
     # ./lib/pubid/ieee/identifier/base.rb:112:in `to_s'
     # ./spec/pubid_ieee/identifiers_parsing_spec.rb:17:in `block (3 levels) in <module:Ieee>'

  3) Pubid::Ieee::Identifier parse specific identifiers IEC/IEEE 62582-1:2011 Edition 1.0 2011-08 behaves like converts pubid to pubid converts pubid to pubid
     Failure/Error: result += "-#{sprintf('%02d', month)}"

     ArgumentError:
       invalid value for Integer(): "08"
     Shared Example Group: "converts pubid to pubid" called from ./spec/pubid_ieee/identifiers_parsing_spec.rb:699
     # ./lib/pubid/ieee/identifier/base.rb:231:in `sprintf'
     # ./lib/pubid/ieee/identifier/base.rb:231:in `edition'
     # ./lib/pubid/ieee/identifier/base.rb:156:in `parameters'
     # ./lib/pubid/ieee/identifier/base.rb:125:in `dual_identifier'
     # ./lib/pubid/ieee/identifier/base.rb:110:in `to_s'
     # ./spec/pubid_ieee/identifiers_parsing_spec.rb:17:in `block (3 levels) in <module:Ieee>'

This is with:

Resolving dependencies...
Using rake 13.0.6
Using bundler 2.4.15
Using io-console 0.6.0
Using json 2.6.3
Using lightly 0.4.0
Using racc 1.7.1
Using parslet 2.0.0
Using rainbow 3.1.1
Using diff-lcs 1.5.0
Using rexml 3.2.5
Using thor 1.2.2
Using parallel 1.23.0
Using ast 2.4.2
Using ruby-progressbar 1.13.0
Using language_server-protocol 3.17.0.3
Using rspec-support 3.12.1
Using parser 3.2.2.3
Using rspec-core 3.12.2
Using rspec-expectations 3.12.3
Using pubid-core 1.8.5
Using rubocop-ast 1.29.0
Using unicode-display_width 2.4.2
Using reline 0.3.6
Using rspec-mocks 3.12.6
Using regexp_parser 2.8.1
Using rspec 3.12.0
Using irb 1.7.4
Using rubocop 1.54.2
Using debug 1.8.0
Using rubocop-capybara 2.18.0
Using rubocop-factory_bot 2.23.1
Using nokogiri 1.15.3 (arm64-darwin)
Using rubocop-rake 0.6.0
Using rubocop-rspec 2.22.0
Using rubocop-performance 1.18.0
Using pubid-iso 0.5.3
Using pubid-ieee 0.1.1 from source at `.`
Bundle updated!

I am running Ruby 3.2.2.

mico commented 1 year ago

I am running Ruby 3.2.2.

This was helpful! Getting this error in ruby 3.2.2, previous ruby versions (< 3.2) are not affected.