From 53bfcc25ef5f803c5ecc6b9585e0f1bd7b288ef5 Mon Sep 17 00:00:00 2001 From: Michael Miller Date: Sat, 6 Apr 2019 17:19:02 -0600 Subject: [PATCH] Add Ameba and address code linting issues --- .gitlab-ci.yml | 3 ++- README.md | 1 + shard.yml | 5 +++++ spec/dsl/nested_example_group_builder_spec.cr | 2 +- spec/dsl/root_example_group_builder_spec.cr | 2 +- spec/example_iterator_spec.cr | 4 ++-- spec/internals/harness_spec.cr | 2 -- spec/matchers/truthy_matcher_spec.cr | 4 ---- spec/nested_example_group_spec.cr | 10 +++++----- spec/root_example_group_spec.cr | 6 +++--- spec/runnable_example_spec.cr | 4 ++-- spec/runner_spec.cr | 8 ++++---- spec/test_suite_spec.cr | 4 ++-- src/spectator/example_group.cr | 4 ++-- src/spectator/formatting/human_time.cr | 2 +- src/spectator/matchers/array_matcher.cr | 4 ++-- 16 files changed, 33 insertions(+), 32 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index fd81d0c..9b55b7f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -13,8 +13,9 @@ before_script: spec: script: - - crystal tool format --check - crystal spec + - bin/ameba + - crystal tool format --check pages: stage: deploy diff --git a/README.md b/README.md index 32f7304..716d432 100644 --- a/README.md +++ b/README.md @@ -341,6 +341,7 @@ Contributing Please make sure to run `crystal tool format` before submitting. The CI build checks for properly formatted code. +[Ameba](https://github.com/veelenga/ameba) is run to check for code style. Tests must be written for any new functionality. Macros that create types are not as easy to test, diff --git a/shard.yml b/shard.yml index c77f919..369ac5e 100644 --- a/shard.yml +++ b/shard.yml @@ -7,3 +7,8 @@ authors: crystal: 0.27.2 license: MIT + +development_dependencies: + ameba: + github: veelenga/ameba + version: ~> 0.9 diff --git a/spec/dsl/nested_example_group_builder_spec.cr b/spec/dsl/nested_example_group_builder_spec.cr index 9d15dcf..cf41fb6 100644 --- a/spec/dsl/nested_example_group_builder_spec.cr +++ b/spec/dsl/nested_example_group_builder_spec.cr @@ -154,7 +154,7 @@ describe Spectator::DSL::NestedExampleGroupBuilder do it "adds a hook" do hook_called = false builder = Spectator::DSL::NestedExampleGroupBuilder.new("foo") - builder.add_around_each_hook(->(proc : ->) { + builder.add_around_each_hook(->(_proc : ->) { hook_called = true }) root = Spectator::DSL::RootExampleGroupBuilder.new.build(Spectator::Internals::SampleValues.empty) diff --git a/spec/dsl/root_example_group_builder_spec.cr b/spec/dsl/root_example_group_builder_spec.cr index 96a9a7a..b4aeb9d 100644 --- a/spec/dsl/root_example_group_builder_spec.cr +++ b/spec/dsl/root_example_group_builder_spec.cr @@ -143,7 +143,7 @@ describe Spectator::DSL::RootExampleGroupBuilder do it "adds a hook" do hook_called = false builder = Spectator::DSL::RootExampleGroupBuilder.new - builder.add_around_each_hook(->(proc : ->) { + builder.add_around_each_hook(->(_proc : ->) { hook_called = true }) group = builder.build(Spectator::Internals::SampleValues.empty) diff --git a/spec/example_iterator_spec.cr b/spec/example_iterator_spec.cr index 8ec5594..882387d 100644 --- a/spec/example_iterator_spec.cr +++ b/spec/example_iterator_spec.cr @@ -93,7 +93,7 @@ describe Spectator::ExampleIterator do end else Spectator::NestedExampleGroup.new(i.to_s, group, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty).tap do |sub_group| - sub_group.children = Array(Spectator::ExampleComponent).new(5) do |j| + sub_group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(sub_group, Spectator::Internals::SampleValues.empty).tap do |example| expected_examples << example end @@ -113,7 +113,7 @@ describe Spectator::ExampleIterator do PassingExample.new(group, Spectator::Internals::SampleValues.empty) else Spectator::NestedExampleGroup.new(i.to_s, group, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty).tap do |sub_group| - sub_group.children = Array(Spectator::ExampleComponent).new(5) do |j| + sub_group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(sub_group, Spectator::Internals::SampleValues.empty) end end diff --git a/spec/internals/harness_spec.cr b/spec/internals/harness_spec.cr index 9074c18..9a360c0 100644 --- a/spec/internals/harness_spec.cr +++ b/spec/internals/harness_spec.cr @@ -53,7 +53,6 @@ describe Spectator::Internals::Harness do describe "#report_expectation" do context "with a successful result" do it "stores the result" do - error = nil.as(Exception?) expectation = new_satisfied_expectation spy = SpyExample.create do harness = Spectator::Internals::Harness.current @@ -81,7 +80,6 @@ describe Spectator::Internals::Harness do end it "stores the result" do - error = nil.as(Exception?) expectation = new_unsatisfied_expectation spy = SpyExample.create do harness = Spectator::Internals::Harness.current diff --git a/spec/matchers/truthy_matcher_spec.cr b/spec/matchers/truthy_matcher_spec.cr index 91acae9..7ef3970 100644 --- a/spec/matchers/truthy_matcher_spec.cr +++ b/spec/matchers/truthy_matcher_spec.cr @@ -125,7 +125,6 @@ describe Spectator::Matchers::TruthyMatcher do it "contains the \"truthy\"" do value = 42 - label = "everything" partial = new_partial(value) matcher = Spectator::Matchers::TruthyMatcher.new(true) match_data = matcher.match(partial) @@ -145,7 +144,6 @@ describe Spectator::Matchers::TruthyMatcher do it "contains the \"truthy\"" do value = 42 - label = "everything" partial = new_partial(value) matcher = Spectator::Matchers::TruthyMatcher.new(true) match_data = matcher.match(partial) @@ -261,7 +259,6 @@ describe Spectator::Matchers::TruthyMatcher do it "contains the \"falsey\"" do value = 42 - label = "everything" partial = new_partial(value) matcher = Spectator::Matchers::TruthyMatcher.new(false) match_data = matcher.match(partial) @@ -281,7 +278,6 @@ describe Spectator::Matchers::TruthyMatcher do it "contains the \"falsey\"" do value = 42 - label = "everything" partial = new_partial(value) matcher = Spectator::Matchers::TruthyMatcher.new(false) match_data = matcher.match(partial) diff --git a/spec/nested_example_group_spec.cr b/spec/nested_example_group_spec.cr index 63d53ca..0e95838 100644 --- a/spec/nested_example_group_spec.cr +++ b/spec/nested_example_group_spec.cr @@ -28,7 +28,7 @@ def nested_group_with_sub_groups(sub_group_count = 5, example_count = 5) examples = [] of Spectator::Example group.children = Array(Spectator::ExampleComponent).new(sub_group_count) do |i| Spectator::NestedExampleGroup.new(i.to_s, group, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty).tap do |sub_group| - sub_group.children = Array(Spectator::ExampleComponent).new(example_count) do |j| + sub_group.children = Array(Spectator::ExampleComponent).new(example_count) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end @@ -359,7 +359,7 @@ describe Spectator::NestedExampleGroup do root = Spectator::RootExampleGroup.new(Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) group = Spectator::NestedExampleGroup.new("what", root, hooks, Spectator::ExampleConditions.empty) root.children = [group.as(Spectator::ExampleComponent)] - group.children = Array(Spectator::ExampleComponent).new(5) do |i| + group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end @@ -378,7 +378,7 @@ describe Spectator::NestedExampleGroup do root = Spectator::RootExampleGroup.new(Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) group = Spectator::NestedExampleGroup.new("what", root, hooks, Spectator::ExampleConditions.empty) root.children = [group.as(Spectator::ExampleComponent)] - group.children = Array(Spectator::ExampleComponent).new(5) do |i| + group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end @@ -397,7 +397,7 @@ describe Spectator::NestedExampleGroup do root = Spectator::RootExampleGroup.new(hooks, Spectator::ExampleConditions.empty) group = Spectator::NestedExampleGroup.new("what", root, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) root.children = [group.as(Spectator::ExampleComponent)] - group.children = Array(Spectator::ExampleComponent).new(5) do |i| + group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end @@ -416,7 +416,7 @@ describe Spectator::NestedExampleGroup do root = Spectator::RootExampleGroup.new(hooks, Spectator::ExampleConditions.empty) group = Spectator::NestedExampleGroup.new("what", root, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) root.children = [group.as(Spectator::ExampleComponent)] - group.children = Array(Spectator::ExampleComponent).new(5) do |i| + group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end diff --git a/spec/root_example_group_spec.cr b/spec/root_example_group_spec.cr index c0528a9..9ae388e 100644 --- a/spec/root_example_group_spec.cr +++ b/spec/root_example_group_spec.cr @@ -22,7 +22,7 @@ def root_group_with_sub_groups(sub_group_count = 5, example_count = 5) examples = [] of Spectator::Example group.children = Array(Spectator::ExampleComponent).new(sub_group_count) do |i| Spectator::NestedExampleGroup.new(i.to_s, group, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty).tap do |sub_group| - sub_group.children = Array(Spectator::ExampleComponent).new(example_count) do |j| + sub_group.children = Array(Spectator::ExampleComponent).new(example_count) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end @@ -246,7 +246,7 @@ describe Spectator::RootExampleGroup do examples = [] of Spectator::Example hooks = new_hooks(after_all: ->{ called = true; nil }) group = Spectator::RootExampleGroup.new(hooks, Spectator::ExampleConditions.empty) - group.children = Array(Spectator::ExampleComponent).new(5) do |i| + group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end @@ -263,7 +263,7 @@ describe Spectator::RootExampleGroup do examples = [] of Spectator::Example hooks = new_hooks(after_each: ->{ called = true; nil }) group = Spectator::RootExampleGroup.new(hooks, Spectator::ExampleConditions.empty) - group.children = Array(Spectator::ExampleComponent).new(5) do |i| + group.children = Array(Spectator::ExampleComponent).new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).tap do |example| examples << example end diff --git a/spec/runnable_example_spec.cr b/spec/runnable_example_spec.cr index 602dc38..5907454 100644 --- a/spec/runnable_example_spec.cr +++ b/spec/runnable_example_spec.cr @@ -856,7 +856,7 @@ describe Spectator::RunnableExample do root = Spectator::RootExampleGroup.new(Spectator::ExampleHooks.empty, conditions) group = Spectator::NestedExampleGroup.new("what", root, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) root.children = [group.as(Spectator::ExampleComponent)] - result = run_example(FailingExample, group) + run_example(FailingExample, group) called.should be_false end end @@ -1219,7 +1219,7 @@ describe Spectator::RunnableExample do root = Spectator::RootExampleGroup.new(Spectator::ExampleHooks.empty, conditions) group = Spectator::NestedExampleGroup.new("what", root, Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) root.children = [group.as(Spectator::ExampleComponent)] - result = run_example(ErroredExample, group) + run_example(ErroredExample, group) called.should be_false end end diff --git a/spec/runner_spec.cr b/spec/runner_spec.cr index 1fa678b..f3778c2 100644 --- a/spec/runner_spec.cr +++ b/spec/runner_spec.cr @@ -100,7 +100,7 @@ describe Spectator::Runner do end it "#start_example is called for each example" do - group = SpyExample.create_group(5) { |index| nil } + group = SpyExample.create_group(5) { |_| nil } suite = new_test_suite(group) spy = SpyFormatter.new runner = Spectator::Runner.new(suite, spectator_test_config(spy)) @@ -109,7 +109,7 @@ describe Spectator::Runner do end it "passes the correct example to #start_example" do - group = SpyExample.create_group(5) { |index| nil } + group = SpyExample.create_group(5) { |_| nil } suite = new_test_suite(group) spy = SpyFormatter.new runner = Spectator::Runner.new(suite, spectator_test_config(spy)) @@ -125,7 +125,7 @@ describe Spectator::Runner do end it "calls #end_example for each example" do - group = SpyExample.create_group(5) { |index| nil } + group = SpyExample.create_group(5) { |_| nil } suite = new_test_suite(group) spy = SpyFormatter.new runner = Spectator::Runner.new(suite, spectator_test_config(spy)) @@ -175,7 +175,7 @@ describe Spectator::Runner do end it "contains the expected time span" do - group = SpyExample.create_group(5) { |index| nil } + group = SpyExample.create_group(5) { |_| nil } suite = new_test_suite(group) spy = SpyFormatter.new runner = Spectator::Runner.new(suite, spectator_test_config(spy)) diff --git a/spec/test_suite_spec.cr b/spec/test_suite_spec.cr index f36cbf1..24846fc 100644 --- a/spec/test_suite_spec.cr +++ b/spec/test_suite_spec.cr @@ -4,7 +4,7 @@ describe Spectator::TestSuite do describe "#each" do it "yields each example" do group = Spectator::RootExampleGroup.new(Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) - group.children = Array.new(5) do |index| + group.children = Array.new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).as(Spectator::ExampleComponent) end test_suite = Spectator::TestSuite.new(group, Spectator::NullExampleFilter.new) @@ -17,7 +17,7 @@ describe Spectator::TestSuite do it "skips examples not in the filter" do group = Spectator::RootExampleGroup.new(Spectator::ExampleHooks.empty, Spectator::ExampleConditions.empty) - group.children = Array.new(5) do |index| + group.children = Array.new(5) do |_| PassingExample.new(group, Spectator::Internals::SampleValues.empty).as(Spectator::ExampleComponent) end test_suite = Spectator::TestSuite.new(group, Spectator::CompositeExampleFilter.new([] of Spectator::ExampleFilter)) diff --git a/src/spectator/example_group.cr b/src/spectator/example_group.cr index d62f7dd..42a23f3 100644 --- a/src/spectator/example_group.cr +++ b/src/spectator/example_group.cr @@ -95,7 +95,7 @@ module Spectator offset = index # Loop through each child # until one is found to contain the index. - child = children.each do |child| + found = children.each do |child| count = child.example_count # Example groups consider their range to be [0, example_count). # Each child is offset by the total example count of the previous children. @@ -114,7 +114,7 @@ module Spectator # Otherwise, the indexer repeats the process for the next child. # It should be impossible to get nil here, # provided the bounds check and example counts are correct. - child.not_nil![offset] + found.not_nil![offset] end # Checks whether all examples in the group have been run. diff --git a/src/spectator/formatting/human_time.cr b/src/spectator/formatting/human_time.cr index e28692f..0eff947 100644 --- a/src/spectator/formatting/human_time.cr +++ b/src/spectator/formatting/human_time.cr @@ -35,7 +35,7 @@ module Spectator::Formatting days = hours / 24 hours %= 24 - return sprintf("%i days %i:%02i:%02i", days, hours, minutes, int_seconds) + sprintf("%i days %i:%02i:%02i", days, hours, minutes, int_seconds) end end end diff --git a/src/spectator/matchers/array_matcher.cr b/src/spectator/matchers/array_matcher.cr index e49c75b..9a7bc2e 100644 --- a/src/spectator/matchers/array_matcher.cr +++ b/src/spectator/matchers/array_matcher.cr @@ -11,8 +11,8 @@ module Spectator::Matchers values = ExpectedActual.new(expected, label, actual, partial.label) if values.expected.size == values.actual.size index = 0 - values.expected.zip(values.actual) do |expected, actual| - return ContentMatchData.new(index, values) unless expected == actual + values.expected.zip(values.actual) do |expected, element| + return ContentMatchData.new(index, values) unless expected == element index += 1 end IdenticalMatchData.new(values)