-
-
Notifications
You must be signed in to change notification settings - Fork 531
all-your-base: add test generator #510
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 2 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,31 +1,36 @@ | ||
| module BookKeeping | ||
| VERSION = 1 | ||
| VERSION = 2 | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bumped test version since we now expect invalid base / input to raise an error rather than return nil |
||
| end | ||
|
|
||
| class BaseConverter | ||
| def self.convert(base_from, number_array, base_to) | ||
| return if number_array.any?{|number| number < 0 || number >= base_from} | ||
| return if base_from <= 1 || base_to <= 1 | ||
| fail ArgumentError if invalid_inputs?(base_from, number_array, base_to) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to change as little as possible here to get tests to pass. Does this look okay?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's fine. |
||
| return [] unless number_array.any? | ||
| number_in_canonical_base = convert_to_canonical_base(number_array, base_from) | ||
| convert_from_canonical_base(number_in_canonical_base, base_to) | ||
| end | ||
|
|
||
| private | ||
| private_class_method | ||
|
|
||
| def self.invalid_inputs?(base_from, number_array, base_to) | ||
| number_array.any? { |number| number < 0 || number >= base_from } || | ||
| base_from <= 1 || base_to <= 1 | ||
| end | ||
|
|
||
| def self.convert_to_canonical_base(number_array, base) | ||
| total = 0 | ||
| number_array.reverse.each_with_index do |number, index| | ||
| total += number * base ** index | ||
| total += number * base**index | ||
| end | ||
| total | ||
| end | ||
|
|
||
| def self.convert_from_canonical_base(number, base_to) | ||
| result = [] | ||
| current_number = number | ||
| while current_number >= base_to do | ||
| result << current_number % base_to | ||
| current_number = current_number / base_to | ||
| while current_number >= base_to | ||
| result << current_number % base_to | ||
| current_number /= base_to | ||
| end | ||
| result << current_number % base_to | ||
| result.reverse | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| require 'minitest/autorun' | ||
| require_relative 'all_your_base' | ||
|
|
||
| # Test data version: <%= sha1 %> | ||
| class AllYourBaseTest < Minitest::Test<% test_cases.each do |test_case| %> | ||
| def <%= test_case.test_name %> | ||
| <%= test_case.skipped %> | ||
| <%= test_case.workload %> | ||
| end | ||
| <% end %> | ||
|
|
||
| <%= IO.read(XRUBY_LIB + '/bookkeeping.md') %> | ||
| def test_bookkeeping | ||
| skip | ||
| assert_equal <%= version.next %>, BookKeeping::VERSION | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| class AllYourBaseCase < OpenStruct | ||
| def test_name | ||
| 'test_%s' % description.downcase.tr(' -', '_') | ||
| end | ||
|
|
||
| def workload | ||
| indent(4, (assignments + assertion).join("\n")) + "\n" | ||
| end | ||
|
|
||
| def skipped | ||
| index.zero? ? '# skip' : 'skip' | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def indent(size, text) | ||
| text.lines.each_with_object('') do |line, obj| | ||
| obj << (line == "\n" ? line : ' ' * size + line) | ||
| end | ||
| end | ||
|
|
||
| def assignments | ||
| [ | ||
| "digits = #{input_digits}", | ||
| "input_base = #{input_base}", | ||
| "output_base = #{output_base}", | ||
| ] | ||
| end | ||
|
|
||
| def assertion | ||
| return error_assertion unless expected | ||
|
|
||
| [ | ||
| "expected = #{expected}", | ||
| "", | ||
| "converted = BaseConverter.convert(input_base, digits, output_base)", | ||
| "", | ||
| "assert_equal expected, converted,", | ||
| indent(13, error_message), | ||
| ] | ||
| end | ||
|
|
||
| def error_assertion | ||
| [ | ||
| "", | ||
| "assert_raises ArgumentError do", | ||
| " BaseConverter.convert(input_base, digits, output_base)", | ||
| "end", | ||
| ] | ||
| end | ||
|
|
||
| def error_message | ||
| %q("Input base: #{input_base}, output base #{output_base}. " \\) \ | ||
| "\n" + %q("Expected #{expected} but got #{converted}.") | ||
| end | ||
| end | ||
|
|
||
| class AllYourBaseCase::PreProcessor | ||
| class << self | ||
| attr_reader :row | ||
|
|
||
| def call(row) | ||
| @row = row | ||
|
|
||
| row.merge('expected' => expected_value) | ||
| end | ||
|
|
||
| private :row | ||
| private | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not: private
attr_reader :rowMove the private things past the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what we were chatting about on Gitter the other day. In ruby < 2.3.0 private reader methods made like that cause a warning. In 2.3 that warning is removed. Links to some info about this are in the last commit message. Thanks for the feedback!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the last commit, and instead of moving the line, one more line was created.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The warning was removed in ruby 2.3, so in all later versions it isn't there and you won't see it in 2.4. Since we use 2.1.2, it shows up in CI and whatnot. I'm not sure if moving the reader under private but keeping the explicit private with method name will still make a warning. I'll give this a try when I'm home. Sorry for the lack of code samples... aparently the iOS keyboard doesn't have a back tick key? 😞
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copying commit message here so it can be read inline: This change is to remove a warning present in ruby versions
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's what I had originally but it's what causes the warning mentioned above. Ruby versions less than 2.3.0 have a warning when a private attribute is defined unless its made private explicitly with private :method_name
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As it is a warning, and not functionally wrong, I would probably leave it at that. This also means as time goes on and the version of Ruby moves forward, it will eventually take care of itself.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer that the code not emit warnings. If that means temporarily doing some sub-optimal things to keep the interpreter happy then so be it.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Definitely prefer warning free. How will we trigger when it should be "corrected" when the warning would naturally go away? Not that it is an important change...
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it needs an explicit trigger. Next time that code gets edited, whoever modifies it will think it looks wrong, (unjustly) curse @tommyschaefer for being an idiot 😜 and fix it. |
||
| def expected_value | ||
| return row['expected'] if row['expected'] | ||
|
|
||
| if invalid_input_digits? || invalid_bases? | ||
| nil | ||
| elsif row['input_digits'].empty? | ||
| [] | ||
| elsif input_of_zero? | ||
| [0] | ||
| else | ||
| handle_special_cases | ||
| end | ||
| end | ||
|
|
||
| def invalid_input_digits? | ||
| row['input_digits'].any? { |x| x < 0 || x >= row['input_base'] } | ||
| end | ||
|
|
||
| def invalid_bases? | ||
| row['input_base'] <= 1 || row['output_base'] <= 1 | ||
| end | ||
|
|
||
| def input_of_zero? | ||
| row['input_digits'].all? { |x| x == 0 } | ||
| end | ||
|
|
||
| def handle_special_cases | ||
| [4,2] if row['input_digits'] == [0, 6, 0] | ||
| end | ||
| end | ||
| end | ||
|
|
||
| AllYourBaseCases = proc do |data| | ||
| JSON.parse(data)['cases'].map.with_index do |row, i| | ||
| AllYourBaseCase.new( | ||
| AllYourBaseCase::PreProcessor.call(row).merge(index: i), | ||
| ) | ||
| end | ||
| end | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Insti Just wanted to double check that it's okay to remove this based on your comment on #509? Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes. remove this line.