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.

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

Bug on sample dataset for Ruby problem: hamming #32

Closed
wwchen opened this issue Sep 27, 2014 · 17 comments
Closed

Bug on sample dataset for Ruby problem: hamming #32

wwchen opened this issue Sep 27, 2014 · 17 comments

Comments

@wwchen
Copy link

wwchen commented Sep 27, 2014

I hope this is the right forum to submit bug reports on the sample dataset

First off, I am getting a warning:

Warning: you should require 'minitest/autorun' instead.
Warning: or add 'gem "minitest"' before 'require "minitest/autorun"'
From:
  /Users/wchen/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/minitest/autorun.rb:15:in `<top (required)>'
  hamming_test.rb:1:in `<main>'
MiniTest::Unit.autorun is now Minitest.autorun. From /Users/wchen/.rvm/rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/minitest/autorun.rb:19:in `<top (required)>'
MiniTest::Unit::TestCase is now Minitest::Test. From hamming_test.rb:4:in `<main>'

Warning message was suppressed by adding gem "minitest" to my hamming_test.rb

Second,
I believe this test has an incorrect assertion:

  def test_ignores_extra_length_on_other_strand_when_longer
    assert_equal 2, Hamming.compute('AGG', 'AAAACTGACCCACCCCAGG')
  end

Correct me if I'm wrong, but assertion should be equal to zero:

AAAACTGACCCACCCCAGG
                AGG
@kerrizor
Copy link

Related to #31 ?

@kytrinyx
Copy link
Member

Warning message was suppressed by adding gem "minitest" to my hamming_test.rb

This is likely because you already have installed minitest as a gem on your system. If you didn't have it, then adding "gem minitest" would require that you install it, and I'd rather avoid forcing people to install dependencies.

I tend to add "gem 'minitest'" manually myself (as well as "require 'minitest/pride'").

@kerrizor is right, the hamming test suite is incorrect at the moment. It's a known issue, and the correct test would probably raise an ArgumentError.

@kotp
Copy link
Member

kotp commented May 20, 2015

The timing is right for this, as we are working on the hamming exercise. Work through a solution and/or close?

@kytrinyx
Copy link
Member

The hamming part of this has been fixed (it now raises an error).

The larger question applies to all of the problems: should we explicitly require that Minitest be installed as a gem? Is it still shipped as part of Ruby, and if so, what version?

I think it's worth working through this.

I'm leaning towards explicitly adding gem 'minitest', '~> 5.0' or something similar to the problems and then making people install minitest, if it will reduce the friction/confusion around this.

@kotp
Copy link
Member

kotp commented May 20, 2015

This could be managed by using 'Bundlerand having aGemfile` in the exercises. Unsure if that is the best way to go for this project, but it is now almost the 'defacto' for Ruby projects.

@kytrinyx
Copy link
Member

If you don't have bundler, then this would be gem install bundler && bundle install, whereas if we just told you gem install minitest that would be simpler and less indirection.

@kotp
Copy link
Member

kotp commented May 21, 2015

Do we have a script that runs when you first install the exercism gem? (In other words, it could be a dependency of the gem.) A rake test task that looks at your current working directory, and tuns the required test. The infrastructure is already here, I think.

@kytrinyx
Copy link
Member

There's no exercism gem--just the binary which has no other dependencies. A rake task would be nice, but I don't think the infrastructure is in place, unless I'm thinking about this all wrong.

@kotp
Copy link
Member

kotp commented May 22, 2015

Strange. As I have this gem installed:

exercism (0.0.28)

Investigating it now.

@kotp
Copy link
Member

kotp commented May 22, 2015

Yeah... and the dependencies include both minitest and bundler...

spec.add_development_dependency "bundler", "~> 1.3"
spec.add_development_dependency "minitest", '~> 5.0'

Right now, restricted to development. The application dependencies are thus:

  spec.add_dependency "json"
  spec.add_dependency "faraday", ">= 0.8.8"
  spec.add_dependency "thor"
  spec.add_dependency "launchy", "~> 2.3.0"

@kotp
Copy link
Member

kotp commented May 25, 2015

Exercism gem has been deprecated... :) never mind.

@kotp
Copy link
Member

kotp commented May 25, 2015

This should be closed if all there is to do is notify folks to install the gem. The instruction is in the hello/-world/GETTING_STARTED.md file to do the gem install minitest which I think is sufficient.

And this does leave the 'set up an automated testing workflow' environment as a learning exercise for individual study as needed.

@kotp
Copy link
Member

kotp commented May 25, 2015

You mentioned about explicitly requiring a minimum version of Minitest? I think the new name for Minitest::Test does this automatically/naturally, to the extent that the syntax doesn't exist in an older version.

@kytrinyx
Copy link
Member

You mentioned about explicitly requiring a minimum version of Minitest?

Yeah, if people are using a version of Rails that requires minitest 4.x, then it becomes pretty complicated to get this to run without warnings.

I think adding gem install minitest to the instructions, and specifying gem 'minitest', '~>5.0' at the top of the exercise file will ensure that it runs clean.

@kotp
Copy link
Member

kotp commented May 26, 2015

Your comment has me shaving yaks this afternoon and evening. Still investigating implications. I use Bundler, so trying to simulate what people do who use neither a version manager of some kind, to include Bundler.

@kytrinyx
Copy link
Member

I hope those yaks didn't ruin your day!

@kotp
Copy link
Member

kotp commented May 26, 2015

Closing this issue as this is a "Two part" thing and the second part really has nothing to do with the hamming problem. Creating a new issue to track "explicitly require that Minitest be installed as a gem".

@kotp kotp closed this as completed May 26, 2015
gchan pushed a commit to gchan/xruby that referenced this issue Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants