wordy: Generate tests by tommyschaefer · Pull Request #509 · exercism/ruby

tommyschaefer · 2016-12-19T22:32:32Z

Hi there!

I've made a first pass at converting the wordy exercise to use generated tests (#396).

I based the updates on #474, but I still have a few questions:

The original tests use a mixture of the following forms:

question = '...'
assert_equal ...

and

assert_equal 1, WordProblem.new('...').answer

I'm assuming this has to do with not going past 80 characters per line. In my first pass I decided to just use the first form for all of the tests. Is this okay, or should I put some work into switching between the two forms based on the length of the assertion?

Does anything special have to happen since this wasn't originally book-kept?
In the test_add_then_multiply / test_addition_and_multiplication test, the xruby implementation includes a custom error message. Should this error message be removed (as it is currently), or should we add the custom error message to the shared metadata and generate based on that?

Thank you so much for your time 😄 !

tommyschaefer · 2016-12-19T22:35:59Z

+  def test_addition_and_multiplication
    skip
    question = 'What is -3 plus 7 multiplied by -2?'
-    message = 'You should ignore order of precedence. -3 + 7 * -2 = -8, not -17'


This is the custom message that isn't currently generated.

Can you work out how to make the generator add this message yourself, or would you like some hints?

Hints would be perfect, actually 😄 ! I can't quite work out how I would implement this without modifying the shared test data

See my latest comments.

tommyschaefer · 2016-12-19T22:41:01Z

@@ -0,0 +1,20 @@
+#!/usr/bin/env ruby


I know that there was some discussion about removing this in #295. Should it be included in newly generated tests? I wasn't too sure whether a decision had been made 😄

I don't think a conclusion was ever reached, you could try leaving it out and see if anyone notices.

Insti · 2016-12-20T07:01:38Z

@@ -0,0 +1,20 @@
+#!/usr/bin/env ruby
+gem 'minitest', '>= 5.0.0'


This line doesn't need to be here.

Insti · 2016-12-20T07:02:00Z

+require_relative 'wordy'
+
+# Test data version:
+# <%= sha1 %>


These 2 comments should be one line.

Insti · 2016-12-20T07:03:36Z

    skip
-    assert_equal(-11, WordProblem.new('What is -1 plus -10?').answer)
+    question = 'What is -1 plus -10?'
+    assert_equal(-11, WordProblem.new(question).answer)


Why does this one have brackets around the arguments?

Hi there!

I originally thought that it was something enforced by rubocop, but I'm not sure now. Whenever I save the file without the parentheses, I get a warning in Vim ambiguous first argument; put parentheses or a space even after '-' operator (only for cases with a negative number).

I'm assuming that's maybe why the original cases also had parens around negative cases?

I think that's a Rubocop warning, it's not particularly important, but it's better to make all the cases consistent so in this case I'd wrap them all in parentheses. This has the pleasant side effect of making your value_assertion method much simpler.

Ah, yes. That sounds great to me!

Insti · 2016-12-20T07:05:46Z

The original tests use a mixture of the following forms:
(snip)
I decided to just use the first form for all of the tests.

This is a good decision.

Does anything special have to happen since this wasn't originally book-kept?

No, just create a .version 1 (which you have done.)

In the test_add_then_multiply / test_addition_and_multiplication test, the xruby implementation includes a custom error message. Should this error message be removed (as it is currently), or should we add the custom error message to the shared metadata and generate based on that?

The custom error message could be added to the shared metadata, but it is probably better to first implement it here in the xruby wordy generator.

See: https://github.com/exercism/xruby/blob/master/lib/triangle_cases.rb for a (poor) example of preprocessing the common data before creating the tests.

Insti · 2016-12-20T07:17:00Z

-  def test_irrelevant
+  def test_non_math_question
    skip
+    question = 'Who is the President of the United States?'


This question doesn't start with 'What is' but that's probably an issue for the common metadata.

Insti · 2016-12-20T08:11:32Z

@tommyschaefer you might be interested in looking at my proposal for how test generators should work in future: Insti#1

tommyschaefer · 2016-12-20T15:09:45Z

@Insti I took a look and it looks great! I based the interface of the WordyCase class on the discussions on #463. Is there anything I can do to WordyCases to make it easier to convert to the future generator? The thing that jumped out most to me was the inclusion of a workload method, but is there anything else that would make it easier to update?

Thank you so much for your help!

Insti · 2016-12-20T19:00:47Z

Don't worry too much about making this future compatible, the new format is not finalized and there will be a project to review all the generators when it is.

Insti · 2016-12-22T09:22:23Z

+
+WordyCases = proc do |data|
+  JSON.parse(data)['cases'].map.with_index do |row, i|
+    WordyCase.new(row.merge('index' => i))


You can preprocess the json here to add a failure message:

wordy_case = WordyCase.new(row.merge('index' => i, failure_message => nil)) if wordy_case.question = 'What is -3 plus 7 multiplied by -2?' wordy_case.failure_message = 'You should ignore order of precedence. -3 + 7 * -2 = -8, not -17' end wordy_case

(untested, a Github comment box is not an ideal programming environment.)

One thing to bear in mind here is that the failure message should show the actual value returned. (This will need to be inserted in the assertion method)

Insti · 2016-12-22T09:25:34Z

+  def assertion
+    return error_assertion unless expected
+
+    "assert_equal(#{expected}, WordProblem.new(question).answer)"


Then you can add your custom failure message here.

message = failure_message || "Expected #{expected} but got #{result}" "assert_equal(#{expected}, WordProblem.new(question).answer, #{message})"

(This code will not work as-is)

tommyschaefer · 2016-12-22T22:24:11Z

@Insti Fantastic! Thank you so much for the help! I'll make these updates and push them this evening! 😀

tommyschaefer · 2016-12-24T14:34:25Z

@Insti Does this look ready to review to you? If so, would you like me to go ahead and squash the commits? Thank you so much for your time and help!

kotp · 2016-12-27T19:46:04Z

This looks good. You could squash.

tommyschaefer · 2016-12-27T22:52:38Z

Hmm... I'm not sure why this is failing all of the sudden. I don't think its based on these commits.

Relevant output of the log:

$ export BUNDLE_GEMFILE=$PWD/Gemfile
$ ruby --version
ruby 2.1.2p95 (2014-05-08 revision 45877) [x86_64-linux]
$ rvm --version
rvm 1.26.10 (latest-minor) by Wayne E. Seguin <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [https://rvm.io/]
$ bundle --version
Bundler version 1.6.5
$ gem --version
2.4.5
$ bundle install --quiet
Gem::Ext::BuildError: ERROR: Failed to build gem native extension.

    /home/travis/.rvm/rubies/ruby-2.1.2/bin/ruby mkrf_conf.rb 

/home/travis/.rvm/rubies/ruby-2.1.2/bin/ruby -rubygems /home/travis/.rvm/gems/ruby-2.1.2/gems/rake-12.0.0/exe/rake RUBYARCHDIR=/home/travis/.rvm/gems/ruby-2.1.2/extensions/x86_64-linux/2.1.0/rainbow-2.2.0 RUBYLIBDIR=/home/travis/.rvm/gems/ruby-2.1.2/extensions/x86_64-linux/2.1.0/rainbow-2.2.0
(in /home/travis/.rvm/gems/ruby-2.1.2/gems/rainbow-2.2.0)
fatal: Not a git repository (or any of the parent directories): .git
rake aborted!
LoadError: cannot load such file -- rspec/core/rake_task
/home/travis/.rvm/gems/ruby-2.1.2/gems/rainbow-2.2.0/Rakefile:2:in `<top (required)>'
/home/travis/.rvm/gems/ruby-2.1.2/gems/rake-12.0.0/exe/rake:27:in `<main>'
(See full trace by running task with --trace)

rake failed, exit code 1

kotp · 2016-12-27T23:08:25Z

The configuration shows that rvm is version 2.1.2. I don't think this is correct..

{
  "language": "ruby",
  "sudo": false,
  "rvm": "2.1.2",
  "install": [

The above snipped...

rvm:
    version:      "rvm 1.28.0 (master) by Wayne E. Seguin <wayneeseguin@gmail.com>, Michal Papis <mpapis@gmail.com> [https://rvm.io/]"

Shows the version of RVM that I am using that was updated only a few days ago.

kotp · 2016-12-28T04:53:05Z

Can you rebase on master and submit to test the travis-ci?

tommyschaefer · 2016-12-28T06:40:35Z

@kotp Looks like it worked! 😄

kotp · 2016-12-28T08:15:14Z

Ready for a squash again.

Insti · 2016-12-29T09:36:31Z

Thanks for all your work on this @tommyschaefer ❤️

tommyschaefer commented Dec 19, 2016

View reviewed changes

tommyschaefer mentioned this pull request Dec 20, 2016

Exercise tests should be generated #396

Closed

94 tasks

Insti reviewed Dec 20, 2016

View reviewed changes

Insti changed the title ~~Exercise Wordy: Generate Tests~~ wordy: Generate tests Dec 20, 2016

Insti reviewed Dec 20, 2016

View reviewed changes

tommyschaefer mentioned this pull request Dec 20, 2016

all-your-base: add test generator #510

Merged

Insti reviewed Dec 22, 2016

View reviewed changes

tommyschaefer force-pushed the generate-wordy-tests branch from be981a8 to 9aaa8a1 Compare December 27, 2016 21:58

tommyschaefer force-pushed the generate-wordy-tests branch from 6723f75 to 2588a3f Compare December 27, 2016 23:04

tommyschaefer force-pushed the generate-wordy-tests branch from 2588a3f to 753d7af Compare December 27, 2016 23:08

tommyschaefer force-pushed the generate-wordy-tests branch from 753d7af to deddfa3 Compare December 28, 2016 06:38

Generate wordy tests

bb75f81

tommyschaefer force-pushed the generate-wordy-tests branch from deddfa3 to bb75f81 Compare December 28, 2016 14:44

tommyschaefer mentioned this pull request Dec 28, 2016

Add phony target for test in Makefile #518

Merged

Insti merged commit 00f9a48 into exercism:master Dec 29, 2016

		@@ -0,0 +1,20 @@
		#!/usr/bin/env ruby
		gem 'minitest', '>= 5.0.0'

Uh oh!

Conversation

tommyschaefer commented Dec 19, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tommyschaefer Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti commented Dec 20, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti commented Dec 20, 2016

Uh oh!

tommyschaefer commented Dec 20, 2016

Uh oh!

Insti commented Dec 20, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti Dec 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Insti Dec 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tommyschaefer commented Dec 22, 2016

Uh oh!

tommyschaefer commented Dec 24, 2016

Uh oh!

kotp commented Dec 27, 2016

Uh oh!

tommyschaefer commented Dec 27, 2016

Uh oh!

kotp commented Dec 27, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kotp commented Dec 28, 2016

Uh oh!

tommyschaefer commented Dec 28, 2016

Uh oh!

kotp commented Dec 28, 2016

Uh oh!

Insti commented Dec 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

tommyschaefer commented Dec 19, 2016 •

edited

Loading

Insti Dec 20, 2016 •

edited

Loading

tommyschaefer Dec 20, 2016 •

edited

Loading

Insti commented Dec 20, 2016 •

edited

Loading

Insti Dec 22, 2016 •

edited

Loading

Insti Dec 22, 2016 •

edited

Loading

kotp commented Dec 27, 2016 •

edited

Loading