bob: added generator by ajwann · Pull Request #657 · exercism/ruby

ajwann · 2017-05-25T22:53:09Z

references #396

Insti · 2017-05-26T06:17:06Z

+
+  def feedback(text)
+    "Bob hears #{text.inspect}, and.."
+  end


This really belongs in the generator, the feedback should be expanded before it gets added to the test.

Insti · 2017-05-26T06:17:50Z

+    "Bob hears #{text.inspect}, and.."
+  end
+
+<% test_cases.each_with_index do |test_case, idx| %>


Use index instead of idx

I agree with @Insti use either index or use i. I prefer i in block argument lists when it is used as an index, otherwise I prefer fully expressing the idea as a word.

I'm totally fine with either. Index is more explicit, so I'll go with that.

Insti · 2017-05-26T06:18:17Z

+class <%= exercise_name_camel %>Test < Minitest::Test
+  def bob
+    Bob
+  end


What is the point of this?

I honestly just tried to reproduce the test pretty much in it's entirety. I agree this method is especially useless. I'm happy to remove it forever.

Insti · 2017-05-26T06:19:25Z

Looking good @ajwann, you're getting good at these.

ajwann · 2017-05-30T22:39:23Z

@Insti I made changes based on you and @kotp's feedback. I also got rid of the call to inspect altogether when interpolating the input in the workload method. Unless I'm missing something it's not necessary anymore.

kotp · 2017-05-31T02:10:16Z

+  def test_other_whitespace
+    skip
+    remark = "
+ 	"


Someones newlines are not being managed in their git configuration?

There's an explicit \n\r(!) in the test data.

I honestly think that working with the various newlines that we have to deal with is an important thing... but why in this exercise. Not appropriate to press back here though... should be addressed in x-common.

Insti · 2017-05-31T05:41:02Z

-    remark = "\t" * rand(1..10)
-    assert_equal 'Fine. Be that way!', bob.hey(remark), feedback(remark)
+    remark = "										"
+    assert_equal 'Fine. Be that way!', Bob.hey(remark), "Bob hears 										, and.."


The things that Bob hears should be quoted.
%Q can be good for this to help avoid needing to quote quotes.

Insti · 2017-05-31T06:02:02Z

The inspects are essential in this exercise due to all the wacky whitespace characters present in the canonical data.

ajwann · 2017-06-05T23:34:01Z

I think we are good to go here.

Insti · 2017-06-07T05:52:23Z

-    remark = "\t" * rand(1..10)
-    assert_equal 'Fine. Be that way!', bob.hey(remark), feedback(remark)
+    remark = %Q(										)
+    assert_equal 'Fine. Be that way!', Bob.hey(remark), %Q(Bob hears 										, and..)


Still needs quotes around the string Bob hears. (input)
You didn't put the inspects back?

Insti · 2017-06-07T05:54:53Z

+
+class BobCase < Generator::ExerciseCase
+  def workload
+    indent_lines(["remark = %Q(#{input})",


Just use inspect here rather than %Q

Insti · 2017-06-07T05:56:02Z

+class BobCase < Generator::ExerciseCase
+  def workload
+    indent_lines(["remark = %Q(#{input})",
+      "assert_equal '#{expected}', Bob.hey(remark), %Q(Bob hears #{input}, and..)"


%Q is right to use here though.
But add an inspect.

"assert_equal '#{expected}', Bob.hey(remark), %Q(Bob hears #{input.inspect}, and..)"

@Insti Adding inspect here actually causes several tests to close the %Q() call early, because those test cases contain literal ) in the actual values. This causes a syntax error on line 101 of the generated test.

If I do something like 'Bob hears #{input.inspect}, and..' then I get the same issue because certain output contains '.

I'm basically trapped in a big catch 22.

👀

I will look soon if @Insti doesn't beat me to it.

Try doing the same with the %Q but instead use %\Q to escape it. That should reconstruct the invocable line, but ignore it in place.

Let me know.

%Q can use different delimiters.
https://ruby-doc.org/core-2.2.0/doc/syntax/literals_rdoc.html#label-Percent+Strings

Would this work?

def workload indent_lines(["remark = #{input.inspect}", "assert_equal '#{expected}', Bob.hey(remark), %q{Bob hears #{input.inspect}, and..}" ], 4) end

lowercase %q is better since we don't want it interpolating the string. (the second time)

@Insti using %q with curly brackets insted of parens did the trick. I had no idea you could use alternative delimiters, thanks!

Insti · 2017-06-07T05:58:05Z

+  def test_other_whitespace
+    skip
+    remark = %Q(
+ 	)


Using inspect should also make the silly whitespace characters usefully visible.

Insti

Thanks @ajwann, we're nearly there, just need to put the inspects back.

ajwann · 2017-06-08T23:01:38Z

The calls to inspect are back and the whitespace characters now show up when the test is generated.

Insti · 2017-06-08T23:10:44Z

+require_relative '<%= exercise_name %>'
+
+# Common test data version: <%= canonical_data_version %> <%= abbreviated_commit_hash %>
+class <%= exercise_name_camel %>Test < Minitest::Test


This looks just like the default test template, are there any changes or can it be deleted?

No changes, it got copied when I copied the generator directory. I'll get rid of it.

Insti

I think this is done. 👍

Insti · 2017-06-10T07:28:24Z

Thanks for all your work on this @ajwann ❤️ ⭐️

Insti reviewed May 26, 2017

View reviewed changes

Insti added the in progress label May 28, 2017

ajwann force-pushed the add-generator-for-bob branch from 2ed109e to 2448c84 Compare May 30, 2017 22:35

kotp reviewed May 31, 2017

View reviewed changes

Insti reviewed May 31, 2017

View reviewed changes

ajwann force-pushed the add-generator-for-bob branch from 2448c84 to 35e151d Compare June 5, 2017 23:31

Insti reviewed Jun 7, 2017

View reviewed changes

Insti suggested changes Jun 7, 2017

View reviewed changes

ajwann force-pushed the add-generator-for-bob branch from 35e151d to 7c7db85 Compare June 8, 2017 22:59

Insti reviewed Jun 8, 2017

View reviewed changes

bob: added generator

81184b1

ajwann force-pushed the add-generator-for-bob branch from 7c7db85 to 81184b1 Compare June 8, 2017 23:30

Insti mentioned this pull request Jun 9, 2017

Add book keeping to Bob problem test #670

Closed

Insti approved these changes Jun 9, 2017

View reviewed changes

kotp merged commit 11b3b41 into exercism:master Jun 10, 2017

Insti mentioned this pull request Oct 1, 2017

Exercise tests should be generated #396

Closed

94 tasks

Uh oh!

Conversation

ajwann commented May 25, 2017 • 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!

kotp May 28, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

Insti commented May 26, 2017

Uh oh!

ajwann commented May 30, 2017 • edited by kotp 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 May 31, 2017 • 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 May 31, 2017

Uh oh!

ajwann commented Jun 5, 2017

Uh oh!

Insti Jun 7, 2017 • 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!

Insti Jun 7, 2017 • 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!

ajwann Jun 8, 2017 • 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!

kotp Jun 8, 2017 • 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 Jun 8, 2017 • 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!

Insti Jun 8, 2017 • 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 left a comment

Choose a reason for hiding this comment

Uh oh!

ajwann commented Jun 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ajwann Jun 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

ajwann commented May 25, 2017 •

edited

Loading

kotp May 28, 2017 •

edited

Loading

ajwann commented May 30, 2017 •

edited by kotp

Loading

Insti May 31, 2017 •

edited

Loading

Insti Jun 7, 2017 •

edited

Loading

Insti Jun 7, 2017 •

edited

Loading

ajwann Jun 8, 2017 •

edited

Loading

kotp Jun 8, 2017 •

edited

Loading

Insti Jun 8, 2017 •

edited

Loading

Insti Jun 8, 2017 •

edited

Loading

ajwann Jun 8, 2017 •

edited

Loading