https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Bug ID: 1268696 Summary: Review Request: rubygem-guard-rspec - Guard gem for RSpec Product: Fedora Version: rawhide Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: ilya.gradina@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org
Spec URL: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-... SRPM URL: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-... Description: Guard::RSpec automatically run your specs (much like autotest). Fedora Account System Username: ilgrad
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Ilya Gradina ilya.gradina@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1268695
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1268695 [Bug 1268695] Review Request: rubygem-guard-compat - Tools for developing Guard compatible plugins
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Ilya Gradina ilya.gradina@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Depends On| |1516328
Referenced Bugs:
https://bugzilla.redhat.com/show_bug.cgi?id=1516328 [Bug 1516328] Review Request: rubygem-guard-compat - Tools for developing Guard compatible plugins
https://bugzilla.redhat.com/show_bug.cgi?id=1268696 Bug 1268696 depends on bug 1268695, which changed state.
Bug 1268695 Summary: Review Request: rubygem-guard-compat - Tools for developing Guard compatible plugins https://bugzilla.redhat.com/show_bug.cgi?id=1268695
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |DUPLICATE
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Jaroslav Prokop jar.prokop@volny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jar.prokop@volny.cz
--- Comment #1 from Jaroslav Prokop jar.prokop@volny.cz --- Hi Ilya, I did an informal review on this package.
* Outdated version - You should make it up to date with upstream release and also update it to more current fedora releases
* Group is not needed - Group tags are not needed currently, so you should delete lines: ~~~ Group: Development/Languages ~~~
~~~ Group: Documentation ~~~
In my opinion you could run the tests without -Ilib option.
Otherwise the package looks good.
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #2 from Jaroslav Prokop jar.prokop@volny.cz --- I am sorry, I forgot one last thing
* Unnecessary dependency - The rubygem "launchy" does not seem like dependency to guard-rspec so I would drop it from requires if possible.
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #3 from Ilya Gradina ilya.gradina@gmail.com --- Hi Jaroslav, thx for the review. rubygem(launchy) - needed for tests, also as rubygem(gem_isolator)(I'll try to send soon on review request).
results build on copr(with disable tests): https://copr.fedorainfracloud.org/coprs/ilgrad/test_rubygems/build/684766/
new spec: https://raw.githubusercontent.com/ilgrad/fedora-packages/master/rubygems/rub... new srpm: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-...
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Vít Ondruch vondruch@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |vondruch@redhat.com
--- Comment #4 from Vít Ondruch vondruch@redhat.com --- BTW I got "warning: bogus date in %changelog: Tue Dec 05 2018 Ilya Gradina ilya.gradina@gmail.com - 4.7.3-1"
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #5 from Vít Ondruch vondruch@redhat.com --- (In reply to Ilya Gradina from comment #3)
also as rubygem(gem_isolator)(I'll try to send soon on review request).
I would not mind if you disabled the specific test case ATM:
~~~ # Requires rubygem(gem_isolator). mv spec/acceptance/formatter_spec.rb{,.disabled} ~~~
But:
1) Of course having gem_isolator in Fedora is better 2) There are another 4 test failures due to "uninitialized constant Bundler" errors. It seems it would be better to disable these test cases, otherwise you will need to fight with all the other dependencies specified in Gemfiles. Not sure ...
https://bugzilla.redhat.com/show_bug.cgi?id=1268696 Bug 1268696 depends on bug 1516328, which changed state.
Bug 1516328 Summary: Review Request: rubygem-guard-compat - Tools for developing Guard compatible plugins https://bugzilla.redhat.com/show_bug.cgi?id=1516328
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |CLOSED Resolution|--- |RAWHIDE
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Jaroslav Prokop jar.prokop@volny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |jar.prokop@volny.cz
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
Jaroslav Prokop jar.prokop@volny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Flags| |fedora-review?
--- Comment #6 from Jaroslav Prokop jar.prokop@volny.cz --- Hi, I'll be taking this for a review. Have you advanced with the package, Ilya?
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #7 from Ilya Gradina ilya.gradina@gmail.com --- (In reply to Jaroslav Prokop from comment #6)
Hi, I'll be taking this for a review. Have you advanced with the package, Ilya?
Thanks Jaroslav,
I'll send a new version of the package soon.
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #8 from Ilya Gradina ilya.gradina@gmail.com --- new spec: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-...
new srpm: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-...
tests do not all pass. Finished in 0.32727 seconds (files took 0.20452 seconds to load) 162 examples, 9 failures
I disabled tests with require gem_isolator, on the advice of Vit.
results tests: https://paste.fedoraproject.org/paste/pvBj2aiPMZgBuK3IZRdpYA
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #9 from Vít Ondruch vondruch@redhat.com --- Oh, you are applying the latest packaging techniques (unpacking the gem via %setup), very nice :)
However, the "rspec -Ilib spec ||:" is too hand-wavy. If the test suite fails entirely, you won't notice. I would rather see the tests disabled one by one or some explanation at minimum.
Why are the Guard::RSpecFormatter tests failing anyway? It seems the time is now formatted differently. Is it Ruby 2.5 thing? Or some incompatibility with latest RSpec? Isn't there patch available upstream already?
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #10 from Ilya Gradina ilya.gradina@gmail.com --- (In reply to Vít Ondruch from comment #9)
Oh, you are applying the latest packaging techniques (unpacking the gem via %setup), very nice :)
However, the "rspec -Ilib spec ||:" is too hand-wavy. If the test suite fails entirely, you won't notice. I would rather see the tests disabled one by one or some explanation at minimum.
Why are the Guard::RSpecFormatter tests failing anyway? It seems the time is now formatted differently. Is it Ruby 2.5 thing? Or some incompatibility with latest RSpec? Isn't there patch available upstream already?
Hi Vit, thanks for the remarks. I had two kinds of errors: Guard::RSpec::RSpecProcess, Guard::RSpecFormatter. Option "rspec --tag ~..." did not work for me, so I used "--exclude-pattern".
new spec: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-... new srpm: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard-...
also there is open issue with Guard::RSpecFormatter: https://github.com/guard/guard-rspec/issues/405
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #11 from Jaroslav Prokop jar.prokop@volny.cz --- The Guard::RSpec::RSpecProcess failures are happening because bundler is needed for them and the upstream does not seem to mention that or require it at all in the file. I recommend notifying upstream about this. For the time being I would probably just comment out the specific test cases.
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #12 from Ilya Gradina ilya.gradina@gmail.com --- (In reply to Jaroslav Prokop from comment #11)
The Guard::RSpec::RSpecProcess failures are happening because bundler is needed for them and the upstream does not seem to mention that or require it at all in the file. I recommend notifying upstream about this. For the time being I would probably just comment out the specific test cases.
ok, thx.
https://bugzilla.redhat.com/show_bug.cgi?id=1268696
--- Comment #13 from Vít Ondruch vondruch@redhat.com --- (In reply to Jaroslav Prokop from comment #11)
The Guard::RSpec::RSpecProcess failures are happening because bundler is needed for them and the upstream does not seem to mention that or require it at all in the file.
Well, upstream is executing the test suite using Bundler, so in that case Bundler is always loaded. The question is if we want to include Bundler as build dependency or not. Both options have their pros/cons.
(In reply to Ilya Gradina from comment #10)
(In reply to Vít Ondruch from comment #9)
Why are the Guard::RSpecFormatter tests failing anyway? It seems the time is now formatted differently. Is it Ruby 2.5 thing? Or some incompatibility with latest RSpec? Isn't there patch available upstream already?
Hi Vit, thanks for the remarks. I had two kinds of errors: Guard::RSpec::RSpecProcess, Guard::RSpecFormatter. Option "rspec --tag ~..." did not work for me, so I used "--exclude-pattern".
new spec: https://github.com/ilgrad/fedora-packages/raw/master/rubygems/rubygem-guard- rspec.spec
You still don't have any remark in the .spec file why you exclude the tests :( That is always very essential to understand why something was done.
also there is open issue with Guard::RSpecFormatter: https://github.com/guard/guard-rspec/issues/405
This does not appear to be related. I'd say that a bit more related is
https://github.com/guard/guard-rspec/pull/403
because it enables upstream to test against more recent versions of Ruby, although it still does not include tests against Ruby 2.5 or ruby-head :(
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com -
Red Hat Bugzilla