From ecad80a96b069b4354b8b5ede02011bad4edc859 Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Wed, 3 Feb 2021 09:49:00 +0200 Subject: [PATCH 1/2] NewRule: SpecFocus closes #172 --- spec/ameba/rule/lint/spec_focus_spec.cr | 127 ++++++++++++++++++++++++ src/ameba/rule/lint/spec_focus.cr | 70 +++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 spec/ameba/rule/lint/spec_focus_spec.cr create mode 100644 src/ameba/rule/lint/spec_focus.cr diff --git a/spec/ameba/rule/lint/spec_focus_spec.cr b/spec/ameba/rule/lint/spec_focus_spec.cr new file mode 100644 index 00000000..328d6f06 --- /dev/null +++ b/spec/ameba/rule/lint/spec_focus_spec.cr @@ -0,0 +1,127 @@ +require "../../../spec_helper" + +module Ameba::Rule::Lint + describe SpecFocus do + subject = SpecFocus.new + + it "does not report if spec is not focused" do + s = Source.new %( + context "context" {} + describe "describe" {} + it "it" {} + pending "pending" {} + ), path: "source_spec.cr" + + subject.catch(s).should be_valid + end + + it "reports if there is a focused context" do + s = Source.new %( + context "context", focus: true do + end + ), path: "source_spec.cr" + + subject.catch(s).should_not be_valid + end + + it "reports if there is a focused describe block" do + s = Source.new %( + describe "describe", focus: true do + end + ), path: "source_spec.cr" + + subject.catch(s).should_not be_valid + end + + it "reports if there is a focused it block" do + s = Source.new %( + it "it", focus: true do + end + ), path: "source_spec.cr" + + subject.catch(s).should_not be_valid + end + + it "reports if there is a focused pending block" do + s = Source.new %( + pending "pending", focus: true do + end + ), path: "source_spec.cr" + + subject.catch(s).should_not be_valid + end + + it "reports if there is a spec item with `focus: false`" do + s = Source.new %( + it "it", focus: false do + end + ), path: "source_spec.cr" + + subject.catch(s).should_not be_valid + end + + it "does not report if there is non spec block with :focus" do + s = Source.new %( + some_method "foo", focus: true do + end + ), path: "source_spec.cr" + + subject.catch(s).should be_valid + end + + it "does not report if there is a tagged item with :focus" do + s = Source.new %( + it "foo", tags: "focus" do + end + ), path: "source_spec.cr" + + subject.catch(s).should be_valid + end + + it "does not report if there are focused spec items without blocks" do + s = Source.new %( + describe "foo", focus: true + context "foo", focus: true + it "foo", focus: true + pending "foo", focus: true + ), path: "source_spec.cr" + + subject.catch(s).should be_valid + end + + it "does not report if there are focused items out of spec file" do + s = Source.new %( + describe "foo", focus: true {} + context "foo", focus: true {} + it "foo", focus: true {} + pending "foo", focus: true {} + ) + + subject.catch(s).should be_valid + end + + it "reports rule, pos and message" do + s = Source.new %( + it "foo", focus: true do + it "bar", focus: true {} + end + ), path: "source_spec.cr" + + subject.catch(s).should_not be_valid + + s.issues.size.should eq(2) + + first, second = s.issues + + first.rule.should_not be_nil + first.location.to_s.should eq "source_spec.cr:1:11" + first.end_location.to_s.should eq "" + first.message.should eq "Focused spec item detected" + + second.rule.should_not be_nil + second.location.to_s.should eq "source_spec.cr:2:13" + second.end_location.to_s.should eq "" + second.message.should eq "Focused spec item detected" + end + end +end diff --git a/src/ameba/rule/lint/spec_focus.cr b/src/ameba/rule/lint/spec_focus.cr new file mode 100644 index 00000000..6407fa11 --- /dev/null +++ b/src/ameba/rule/lint/spec_focus.cr @@ -0,0 +1,70 @@ +module Ameba::Rule::Lint + # Checks if specs are focused. + # + # In specs `focus: true` is mainly used to focus on a spec + # item locally during development. However, if such change + # is committed, it silently runs only focused spec on all + # other enviroment, which is undesired. + # + # This is considered bad: + # + # ``` + # describe MyClass, focus: true do + # end + # + # describe ".new", focus: true do + # end + # + # context "my context", focus: true do + # end + # + # it "works", focus: true do + # end + # ``` + # + # And it should be written as the following: + # + # ``` + # describe MyClass do + # end + # + # describe ".new" do + # end + # + # context "my context" do + # end + # + # it "works" do + # end + # ``` + # + # YAML configuration example: + # + # ``` + # Lint/SpecFocus: + # Enabled: true + # ``` + class SpecFocus < Base + properties do + description "Reports focused spec items" + end + + MSG = "Focused spec item detected" + SPEC_ITEM_NAMES = %w(describe context it pending) + + def test(source) + return unless source.path.ends_with?("_spec.cr") + + AST::NodeVisitor.new self, source + end + + def test(source, node : Crystal::Call) + return unless node.name.in?(SPEC_ITEM_NAMES) + return unless node.block + + arg = node.named_args.try &.find(&.name.== "focus") + + issue_for arg, MSG if arg + end + end +end From f8c22a6e77b839eda99cfe6df1ce5f2803d27e74 Mon Sep 17 00:00:00 2001 From: Vitalii Elenhaupt Date: Wed, 3 Feb 2021 17:14:03 +0200 Subject: [PATCH 2/2] Utilize Source#spec? --- spec/ameba/source_spec.cr | 12 ++++++++++++ src/ameba/rule/lint/spec_focus.cr | 2 +- src/ameba/source.cr | 7 ++++++- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/spec/ameba/source_spec.cr b/spec/ameba/source_spec.cr index cd8af875..fd4781fa 100644 --- a/spec/ameba/source_spec.cr +++ b/spec/ameba/source_spec.cr @@ -23,6 +23,18 @@ module Ameba end end + describe "#spec?" do + it "returns true if the source is a spec file" do + s = Source.new "", "./source_spec.cr" + s.spec?.should be_true + end + + it "returns false if the source is not a spec file" do + s = Source.new "", "./source.cr" + s.spec?.should be_false + end + end + describe "#matches_path?" do it "returns true if source's path is matched" do s = Source.new "", "source.cr" diff --git a/src/ameba/rule/lint/spec_focus.cr b/src/ameba/rule/lint/spec_focus.cr index 6407fa11..d1810e31 100644 --- a/src/ameba/rule/lint/spec_focus.cr +++ b/src/ameba/rule/lint/spec_focus.cr @@ -53,7 +53,7 @@ module Ameba::Rule::Lint SPEC_ITEM_NAMES = %w(describe context it pending) def test(source) - return unless source.path.ends_with?("_spec.cr") + return unless source.spec? AST::NodeVisitor.new self, source end diff --git a/src/ameba/source.cr b/src/ameba/source.cr index 2e3f2af4..68dd2a1c 100644 --- a/src/ameba/source.cr +++ b/src/ameba/source.cr @@ -53,7 +53,12 @@ module Ameba File.expand_path(path) end - # Returns true if *filepath* matches the source's path, false if it does not. + # Returns `true` if the source is a spec file, `false` otherwise. + def spec? + path.ends_with?("_spec.cr") + end + + # Returns `true` if *filepath* matches the source's path, `false` if it does not. def matches_path?(filepath) path == filepath || path == File.expand_path(filepath) end