better tatl_tael config, refs SD-2295

test plan:
* see test commits

Change-Id: If9f8cf898cdc76e81196de1f5bf8c91fea098126
Reviewed-on: https://gerrit.instructure.com/110408
Tested-by: Jenkins
Reviewed-by: Jon Jensen <jon@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
This commit is contained in:
Landon Wilkins 2017-05-01 14:30:53 -06:00
parent 11c206dee7
commit 3e96d3232c
23 changed files with 755 additions and 608 deletions

View File

@ -0,0 +1,147 @@
---
RubySpecsLinter:
Severity: "warn"
Globs:
Ruby:
# TODO: gems, plugins, etc?
- "app/**.rb"
- "lib/**.rb"
RubySpec: # excludes SeleniumSpec globs
# canvas proper
- "spec/**.rb"
- "spec_canvas/**.rb"
- "test/**.rb"
# gems, plugins, etc.
- "**/spec/**.rb"
- "**/spec_canvas/**.rb"
- "**/test/**.rb"
SeleniumSpec:
# canvas proper
- "spec/selenium/**"
- "spec_canvas/selenium/**"
- "test/selenium/**"
# gems, plugins, etc.
- "**/spec/selenium/**"
- "**/spec_canvas/selenium/**"
- "**/test/selenium/**"
Messages:
RubyChangesWithOnlySelenium: Your commit includes ruby changes, but does not include
non-selenium specs (model, controller, etc). Please add some to verify your changes.
RubyChangesWithNoRubySpecs: Your commit includes ruby changes, but does not include ruby specs.
Please add some to verify your changes.
SeleniumSpecsLinter:
Severity: "warn"
Globs:
Ruby:
# TODO: gems, plugins, etc?
- "app/**.rb"
- "lib/**.rb"
RubySpec: # excludes SeleniumSpec globs
# canvas proper
- "spec/**.rb"
- "spec_canvas/**.rb"
- "test/**.rb"
# gems, plugins, etc.
- "**/spec/**.rb"
- "**/spec_canvas/**.rb"
- "**/test/**.rb"
SeleniumSpec:
# canvas proper
- "spec/selenium/**"
- "spec_canvas/selenium/**"
- "test/selenium/**"
# gems, plugins, etc.
- "**/spec/selenium/**"
- "**/spec_canvas/selenium/**"
- "**/test/selenium/**"
# the following globs are used to determine if there are unnecessary selenium specs.
# i.e., the code changes could/should be tested at a lower level.
PublicJs:
- "public/javascripts/**.js"
PublicJsWhitelist:
- "**/bower/**"
- "**/mediaelement/**"
- "**/shims/**"
- "**/vendor/**"
- "**/symlink_to_node_modules/**"
PublicJsSpec:
- "spec/coffeescripts/**"
- "spec/javascripts/**"
Coffee:
- "app/coffeescripts/**.coffee"
CoffeeWhitelist:
- "app/coffeescripts/bundles/**"
CoffeeSpec:
- "spec/coffeescripts/**.coffee"
Jsx:
- "app/jsx/**.js"
JsxSpec:
- "spec/coffeescripts/jsx/**"
- "spec/javascripts/jsx/**"
Message: Your commit modifies selenium specs, when your changes might be more appropriately
tested at a lower level. Please limit your selenium specs to happy-path scenarios.
Simple/CoffeeSpecsLinter:
Severity: "warn"
Precondition:
Include:
- "app/coffeescripts/**.coffee"
Whitelist:
- "app/coffeescripts/bundles/**"
Requirement:
Include:
- "spec/coffeescripts/**.coffee"
- "spec/coffeescripts/jsx/**"
- "spec/javascripts/jsx/**"
Message: Your commit includes coffee changes, but does not include coffee or jsx
specs. Please add some to verify your changes.
Simple/JsxSpecsLinter:
Severity: "warn"
Precondition:
Include:
- "app/jsx/**.js"
Requirement:
Include:
- "spec/coffeescripts/jsx/**"
- "spec/javascripts/jsx/**"
Message: Your commit includes coffee changes, but does not include coffee or jsx
specs. Please add some to verify your changes.
Simple/PublicJsSpecsLinter:
Severity: "warn"
Precondition:
Include:
- "public/javascripts/**.js"
Whitelist:
- "**/bower/**"
- "**/mediaelement/**"
- "**/shims/**"
- "**/vendor/**"
- "**/symlink_to_node_modules/**"
Requirement:
Include:
- "spec/coffeescripts/**"
- "spec/javascripts/**"
Message: Your commit includes changes to public/javascripts, but does not include
specs (coffee or jsx). Please add some to verify your changes. Even $.fn.crazyMethods
can and should be tested (and not via selenium).
Simple/NewErbLinter:
Severity: "warn"
Precondition:
Include:
- "app/views/**.erb"
Statuses:
- added
Message: |-
Your commit includes new ERB files, which has been a no-no in Canvas since 2011. All new UI should be built in React on top of documented APIs.
Maybe try doing something like this in your controller instead:
@page_title = t('Your Page Title')
@body_classes << 'whatever-classes you-want-to-add-to-body'
js_bundle :your_js_bundle
css_bundle :any_css_bundles_you_want
js_env({whatever: 'you need to put in window.ENV'})
render :text => "".html_safe, :layout => true

View File

@ -1 +1 @@
require_relative 'tatl_tael/linter'
require_relative 'tatl_tael/linters'

View File

@ -1,53 +0,0 @@
module TatlTael
module Linters
class BaseLinter
class << self
def inherited(subclass)
Linters.linters << subclass unless subclass.name =~ /SimpleLinter/
end
end
attr_reader :changes
def initialize(changes)
@changes = changes
end
### core
def changes_matching(statuses: %w[added modified], # excludes "deleted",
include_regexes: [/./], # include everything
exclude_regexes: []) # don't exclude anything
changes.select do |change|
statuses.include?(change.status) &&
include_regexes.any? { |regex| change.path =~ regex } &&
exclude_regexes.all? { |regex| change.path !~ regex }
end
end
# convenience
def changes_exist?(query)
!changes_matching(query).empty?
end
end
class << self
def linters
@linters ||= []
end
def comments(changes)
[
SimpleLinter.comments(changes),
run_linters(changes)
].flatten.compact
end
def run_linters(changes)
@linter_results ||= linters.map do |linter_class|
linter_class.new(changes).run
end.flatten.compact
end
end
end
end
Dir[File.dirname(__FILE__) + "/linters/*_linter.rb"].each { |file| require file }

View File

@ -0,0 +1,89 @@
require 'yaml'
module TatlTael
module Linters
class BaseLinter
class << self
def inherited(subclass)
Linters.linters << subclass unless subclass.name =~ /SimpleLinter/
end
end
attr_reader :changes
attr_reader :config
def initialize(config:, changes:)
@changes = changes
@config = config
end
### core
def changes_matching(statuses: %w[added modified], # excludes "deleted",
include: ["*"], # include everything
whitelist: []) # don't whitelist anything
changes.select do |change|
statuses.include?(change.status) &&
include.any? { |pattern| File.fnmatch(pattern, change.path) } &&
whitelist.all? { |pattern| !File.fnmatch(pattern, change.path) }
end
end
# convenience
def changes_exist?(query)
!changes_matching(query).empty?
end
end
class << self
def linters
@linters ||= []
end
DEFAULT_CONFIG_PATH = File.join(File.dirname(__FILE__), "../../config/default.yml")
def config
@config ||= YAML.load_file(DEFAULT_CONFIG_PATH)
end
def config_for_linter(linter_class)
# example linter_class.to_s: "TatlTael::Linters::Simple::CoffeeSpecsLinter"
# example resulting base_config_key: "Simple/CoffeeSpecsLinter"
base_config_key = linter_class.to_s
.sub(self.to_s, "") # rm "TatlTael::Linters"
.sub("::", "")
.gsub("::", "/")
underscore_and_symbolize_keys(config[base_config_key])
end
def comments(changes)
@comments ||= linters.map do |linter_class|
linter_class.new(config: config_for_linter(linter_class), changes: changes).run
end.flatten.compact
end
def underscore_and_symbolize_keys(hash)
if hash.is_a? Hash
return hash.reduce({}) do |memo, (k, v)|
memo.tap { |m| m[underscore(k).to_sym] = underscore_and_symbolize_keys(v) }
end
end
if hash.is_a? Array
return hash.each_with_object([]) do |v, memo|
memo << underscore_and_symbolize_keys(v)
end
end
hash
end
def underscore(string)
# borrowed from AS underscore, since we may not have it
string.gsub(/([A-Z\d]+)([A-Z][a-z])/, "\\1_\\2")
.gsub(/([a-z\d])([A-Z])/, "\\1_\\2")
.tr("-", "_")
.downcase
end
end
end
end
Dir[File.dirname(__FILE__) + "/linters/**/*_linter.rb"].each { |file| require file }

View File

@ -1,112 +1,47 @@
require_relative "../support/regex"
module TatlTael
module Linters
# TODO: inherit from SimpleLinter
class RubySpecsLinter < BaseLinter
include Regex
attr_reader :config, :comment
SEVERITY = "warn".freeze
COMMENT_DEFAULTS = {
severity: SEVERITY,
cover_message: true
}.freeze
def initialize(config:, changes:)
@config = config
@changes = changes
@comment = {
message: comment_msg,
severity: config[:severity],
cover_message: true
}
end
RUBY_ONLY_SELENIUM_MSG = "Your commit includes ruby changes,"\
" but does not include non-selenium specs (model, controller, etc)."\
" Please add some to verify your changes.".freeze
RUBY_NO_SPECS_MSG = "Your commit includes ruby changes,"\
" but does not include ruby specs."\
" Please add some to verify your changes.".freeze
BAD_SELENIUM_SPEC_MSG = "Your commit modifies selenium specs,"\
" when your changes might be more appropriately"\
" tested at a lower level (see above)."\
" Please limit your selenium specs to happy-path scenarios.".freeze
def comment_msg
return unless missing_ruby_specs?
if selenium_specs?
config[:messages][:ruby_changes_with_only_selenium]
else
config[:messages][:ruby_changes_with_no_ruby_specs]
end
end
def run
comments = []
if missing_ruby_specs?
comments << if selenium_specs?
RUBY_ONLY_SELENIUM_MSG
else
RUBY_NO_SPECS_MSG
end
end
comments << BAD_SELENIUM_SPEC_MSG if unnecessary_selenium_specs?
comments.map { |comment| COMMENT_DEFAULTS.merge({ message: comment }) }
comment if comment_msg
end
def missing_ruby_specs?
needs_ruby_specs? && !ruby_specs?
end
RUBY_REGEX = /(app|lib)\/.*\.rb$/
def needs_ruby_specs?
changes_exist?(include_regexes: [RUBY_REGEX])
changes_exist?(include: config[:globs][:ruby])
end
RUBY_SPEC_REGEX = /(spec|spec_canvas|test)\/.*\.rb$/
SELENIUM_SPEC_REGEX = /(spec|spec_canvas|test)\/selenium\//
def ruby_specs?
changes_exist?(include_regexes: [RUBY_SPEC_REGEX],
exclude_regexes: [SELENIUM_SPEC_REGEX])
changes_exist?(include: config[:globs][:ruby_spec],
whitelist: config[:globs][:selenium_spec])
end
def selenium_specs?
changes_exist?(include_regexes: [SELENIUM_SPEC_REGEX])
end
def unnecessary_selenium_specs?
selenium_specs? && needs_non_selenium_specs?
end
def needs_non_selenium_specs?
missing_public_js_specs? ||
missing_coffee_specs? ||
missing_jsx_specs? ||
missing_ruby_specs?
end
### public js specs
def missing_public_js_specs?
needs_public_js_specs? && !public_js_specs?
end
def needs_public_js_specs?
changes_exist?(include_regexes: [PUBLIC_JS_REGEX],
exclude_regexes: [PUBLIC_JS_REGEX_EXCLUDE])
end
def public_js_specs?
changes_exist?(include_regexes: [PUBLIC_JS_SPEC_REGEX])
end
### coffee specs
def missing_coffee_specs?
needs_coffee_specs? && !coffee_specs?
end
def needs_coffee_specs?
changes_exist?(include_regexes: [COFFEE_REGEX],
exclude_regexes: [COFFEE_REGEX_EXCLUDE])
end
def coffee_specs?
changes_exist?(include_regexes: [COFFEE_SPEC_REGEX, JSX_SPEC_REGEX])
end
### jsx specs
def missing_jsx_specs?
needs_jsx_specs? && !jsx_specs?
end
def needs_jsx_specs?
changes_exist?(include_regexes: [JSX_REGEX])
end
def jsx_specs?
changes_exist?(include_regexes: [JSX_SPEC_REGEX])
changes_exist?(include: config[:globs][:selenium_spec])
end
end
end

View File

@ -0,0 +1,96 @@
module TatlTael
module Linters
# TODO: inherit from SimpleLinter
class SeleniumSpecsLinter < BaseLinter
attr_reader :config, :comment
def initialize(config:, changes:)
@config = config
@comment = {
message: config[:message],
severity: config[:severity],
cover_message: true
}
@changes = changes
end
def run
comment if unnecessary_selenium_specs?
end
def unnecessary_selenium_specs?
selenium_specs? && needs_non_selenium_specs?
end
# TODO: turn into a "Simple" linter
# Precondition:
# Include: config[:globs][:selenium_spec]
# RequireOne:
def selenium_specs?
changes_exist?(include: config[:globs][:selenium_spec])
end
def needs_non_selenium_specs?
missing_public_js_specs? ||
missing_coffee_specs? ||
missing_jsx_specs? ||
missing_ruby_specs?
end
### public js specs
def missing_public_js_specs?
needs_public_js_specs? && !public_js_specs?
end
def needs_public_js_specs?
changes_exist?(include: config[:globs][:public_js],
whitelist: config[:globs][:public_js_whitelist])
end
def public_js_specs?
changes_exist?(include: config[:globs][:public_js_spec])
end
### coffee specs
def missing_coffee_specs?
needs_coffee_specs? && !coffee_specs?
end
def needs_coffee_specs?
changes_exist?(include: config[:globs][:coffee],
whitelist: config[:globs][:coffee_whitelist])
end
def coffee_specs?
changes_exist?(include: config[:globs][:coffee_spec].concat(config[:globs][:jsx_spec]))
end
### jsx specs
def missing_jsx_specs?
needs_jsx_specs? && !jsx_specs?
end
def needs_jsx_specs?
changes_exist?(include: config[:globs][:jsx])
end
def jsx_specs?
changes_exist?(include: config[:globs][:jsx_spec])
end
### ruby specs (non selenium)
def missing_ruby_specs?
needs_ruby_specs? && !ruby_specs?
end
def needs_ruby_specs?
changes_exist?(include: config[:globs][:ruby])
end
def ruby_specs?
changes_exist?(include: config[:globs][:ruby_spec],
whitelist: config[:globs][:selenium_spec])
end
end
end
end

View File

@ -0,0 +1,10 @@
require_relative "../simple_linter"
module TatlTael
module Linters
module Simple
class CoffeeSpecsLinter < SimpleLinter
end
end
end
end

View File

@ -0,0 +1,10 @@
require_relative "../simple_linter"
module TatlTael
module Linters
module Simple
class JsxSpecsLinter < SimpleLinter
end
end
end
end

View File

@ -0,0 +1,10 @@
require_relative "../simple_linter"
module TatlTael
module Linters
module Simple
class NewErbLinter < SimpleLinter
end
end
end
end

View File

@ -0,0 +1,10 @@
require_relative "../simple_linter"
module TatlTael
module Linters
module Simple
class PublicJsSpecsLinter < SimpleLinter
end
end
end
end

View File

@ -1,19 +1,15 @@
require_relative "../support/regex"
module TatlTael
module Linters
class SimpleLinter < BaseLinter
include Regex
attr_reader :config, :comment
COMMENT_DEFAULTS = {
severity: "warn",
cover_message: true
}.freeze
def initialize(config:, changes:)
@config = config
@comment = COMMENT_DEFAULTS.merge({ message: config[:message] })
@comment = {
message: config[:message],
severity: config[:severity],
cover_message: true
}
@changes = changes
end
@ -28,73 +24,6 @@ module TatlTael
def requirement_met?
config[:requirement] && changes_exist?(config[:requirement])
end
ERB_REGEX = /app\/views\/.*\.erb$/
def self.comments(changes)
configs.flat_map do |config|
new({ config: config, changes: changes }).run
end.compact
end
def self.configs
[
{
name: "CoffeeSpecsLinter",
precondition: {
include_regexes: [COFFEE_REGEX],
exclude_regexes: [COFFEE_REGEX_EXCLUDE],
},
requirement: {
include_regexes: [
COFFEE_SPEC_REGEX,
JSX_SPEC_REGEX
],
},
message: "Your commit includes coffee changes,"\
" but does not include coffee or jsx specs."\
" Please add some to verify your changes."
},
{
name: "JsxSpecsLinter",
precondition: {include_regexes: [JSX_REGEX]},
requirement: {include_regexes: [JSX_SPEC_REGEX]},
message: "Your commit includes coffee changes,"\
" but does not include coffee or jsx specs."\
" Please add some to verify your changes."
},
{
name: "PublicJsSpecsLinter",
precondition: {
include_regexes: [PUBLIC_JS_REGEX],
exclude_regexes: [PUBLIC_JS_REGEX_EXCLUDE],
},
requirement: {include_regexes: [PUBLIC_JS_SPEC_REGEX]},
message: "Your commit includes changes to public/javascripts,"\
" but does not include specs (coffee or jsx)."\
" Please add some to verify your changes."\
" Even $.fn.crazyMethods can and should be tested"\
" (and not via selenium)."
},
{
name: "NewErbLinter",
precondition: {
include_regexes: [ERB_REGEX],
statuses: %w[added]
},
message: "Your commit includes new ERB files,"\
" which has been a no-no in Canvas since 2011."\
" All new UI should be built in React on top of documented APIs.\n"\
"Maybe try doing something like this in your controller instead:\n\n"\
" @page_title = t('Your Page Title')\n"\
" @body_classes << 'whatever-classes you-want-to-add-to-body'\n"\
" js_bundle :your_js_bundle\n"\
" css_bundle :any_css_bundles_you_want\n"\
" js_env({whatever: 'you need to put in window.ENV'})\n"\
" render :text => \"\".html_safe, :layout => true"
}
]
end
end
end
end

View File

@ -1,15 +0,0 @@
module TatlTael
module Linters
module Regex
# shared regexes
COFFEE_REGEX = /app\/coffeescripts\/.*\.coffee$/
COFFEE_REGEX_EXCLUDE = /bundles\//
COFFEE_SPEC_REGEX = /spec\/coffeescripts\//
JSX_REGEX = /app\/jsx\/.*\.js/ # (no longer .jsx ending)
JSX_SPEC_REGEX = /spec\/(coffeescripts|javascripts)\/jsx\//
PUBLIC_JS_REGEX = /public\/javascripts\/.*\.js$/
PUBLIC_JS_REGEX_EXCLUDE = /(bower|mediaelement|shims|vendor|symlink_to_node_modules)\//
PUBLIC_JS_SPEC_REGEX = /spec\/(coffeescripts|javascripts)\//
end
end
end

View File

@ -0,0 +1,24 @@
require 'spec_helper'
require_relative "./shared_constants"
require_relative "./shared_linter_examples"
describe TatlTael::Linters::RubySpecsLinter do
let(:linter_class) { described_class }
let(:config) { TatlTael::Linters.config_for_linter(described_class) }
describe "ensure ruby specs" do
context "app" do
include_examples "change combos with msg key",
Consts::APP_RB_PATH,
Consts::APP_RB_SPEC_PATH,
:ruby_changes_with_no_ruby_specs
end
context "lib" do
include_examples "change combos with msg key",
Consts::LIB_RB_PATH,
Consts::LIB_RB_SPEC_PATH,
:ruby_changes_with_no_ruby_specs
end
end
end

View File

@ -1,75 +1,10 @@
require 'spec_helper'
require_relative "./shared_constants"
require_relative "./shared_linter_examples"
describe TatlTael::Linters::RubySpecsLinter do
describe TatlTael::Linters::SeleniumSpecsLinter do
let(:linter_class) { described_class }
shared_examples "comments" do |raw_changes, msg|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter) { described_class.new(changes) }
it "comments" do
allow(linter_class).to receive(:new)
.with(changes)
.and_return(linter)
result = linter.run.select { |comment| comment[:message] == msg }
expect(result.size).to eq(1)
expect(result.first[:message]).to eq(msg)
end
end
shared_examples "does not comment" do |raw_changes|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter) { described_class.new(changes) }
it "does not comment" do
allow(linter_class).to receive(:new)
.with(changes)
.and_return(linter)
expect(linter.run).to be_empty
end
end
shared_examples "change combos" do |change_path, spec_path, msg|
context "not deletion" do
context "no spec changes" do
include_examples "comments",
[{path: change_path, status: "added"}],
msg
end
context "has spec non deletions" do
include_examples "does not comment",
[{path: change_path, status: "modified"},
{path: spec_path, status: "added"}]
end
context "has spec deletions" do
include_examples "comments",
[{path: change_path, status: "added"},
{path: spec_path, status: "deleted"}],
msg
end
end
context "deletion" do
include_examples "does not comment",
[{path: change_path, status: "deleted"}]
end
end
describe "ensure ruby specs" do
context "app" do
include_examples "change combos",
Consts::APP_RB_PATH,
Consts::APP_RB_SPEC_PATH,
TatlTael::Linters::RubySpecsLinter::RUBY_NO_SPECS_MSG
end
context "lib" do
include_examples "change combos",
Consts::LIB_RB_PATH,
Consts::LIB_RB_SPEC_PATH,
TatlTael::Linters::RubySpecsLinter::RUBY_NO_SPECS_MSG
end
end
let(:config) { TatlTael::Linters.config_for_linter(described_class) }
context "unnecessary selenium specs" do
context "has selenium specs" do
@ -78,7 +13,7 @@ describe TatlTael::Linters::RubySpecsLinter do
include_examples "comments",
[{path: Consts::SELENIUM_SPEC_PATH, status: "added"},
{path: Consts::PUBLIC_JS_PATH, status: "added"}],
TatlTael::Linters::RubySpecsLinter::BAD_SELENIUM_SPEC_MSG
:unnecessary_selenium_specs
end
context "has public js specs" do
@ -94,7 +29,7 @@ describe TatlTael::Linters::RubySpecsLinter do
include_examples "comments",
[{path: Consts::SELENIUM_SPEC_PATH, status: "added"},
{path: Consts::APP_COFFEE_PATH, status: "added"}],
TatlTael::Linters::RubySpecsLinter::BAD_SELENIUM_SPEC_MSG
:unnecessary_selenium_specs
end
context "has coffee specs" do
@ -110,7 +45,7 @@ describe TatlTael::Linters::RubySpecsLinter do
include_examples "comments",
[{path: Consts::SELENIUM_SPEC_PATH, status: "added"},
{path: Consts::APP_JSX_PATH, status: "added"}],
TatlTael::Linters::RubySpecsLinter::BAD_SELENIUM_SPEC_MSG
:unnecessary_selenium_specs
end
context "has jsx specs" do
@ -126,13 +61,13 @@ describe TatlTael::Linters::RubySpecsLinter do
include_examples "comments",
[{path: Consts::SELENIUM_SPEC_PATH, status: "added"},
{path: Consts::APP_RB_PATH, status: "added"}],
TatlTael::Linters::RubySpecsLinter::BAD_SELENIUM_SPEC_MSG
:unnecessary_selenium_specs
# has selenium specs only
include_examples "comments",
[{path: Consts::SELENIUM_SPEC_PATH, status: "added"},
{path: Consts::APP_RB_PATH, status: "added"}],
TatlTael::Linters::RubySpecsLinter::RUBY_ONLY_SELENIUM_MSG
:ruby_changes_with_only_selenium
end
context "has ruby specs" do

View File

@ -0,0 +1,76 @@
shared_examples "comments" do |raw_changes|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter) { described_class.new(changes: changes, config: config) }
it "comments" do
expect(linter.run).to match(hash_including(linter.comment))
end
end
shared_examples "comments with msg key" do |raw_changes, msg_key|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter) { described_class.new(changes: changes, config: config) }
it "comments" do
result = linter.run
expect(result).to match(hash_including(linter.comment))
expect(result[:message]).to eq(config[:messages][msg_key])
end
end
shared_examples "does not comment" do |raw_changes|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter) { described_class.new(changes: changes, config: config) }
it "does not comment" do
expect(linter.run).to be_nil
end
end
shared_examples "change combos" do |change_path, spec_path|
context "not deletion" do
context "no spec changes" do
include_examples "comments",
[{path: change_path, status: "added"}]
end
context "has spec non deletions" do
include_examples "does not comment",
[{path: change_path, status: "modified"},
{path: spec_path, status: "added"}]
end
context "has spec deletions" do
include_examples "comments",
[{path: change_path, status: "added"},
{path: spec_path, status: "deleted"}]
end
end
context "deletion" do
include_examples "does not comment",
[{path: change_path, status: "deleted"}]
end
end
shared_examples "change combos with msg key" do |change_path, spec_path, msg_key|
context "not deletion" do
context "no spec changes" do
include_examples "comments with msg key",
[{path: change_path, status: "added"}],
msg_key
end
context "has spec non deletions" do
include_examples "does not comment",
[{path: change_path, status: "modified"},
{path: spec_path, status: "added"}]
end
context "has spec deletions" do
include_examples "comments with msg key",
[{path: change_path, status: "added"},
{path: spec_path, status: "deleted"}],
msg_key
end
end
context "deletion" do
include_examples "does not comment",
[{path: change_path, status: "deleted"}]
end
end

View File

@ -0,0 +1,24 @@
require 'spec_helper'
require_relative "../shared_constants"
require_relative "../shared_linter_examples"
describe TatlTael::Linters::Simple::CoffeeSpecsLinter do
let(:config) { TatlTael::Linters.config_for_linter(described_class) }
context "coffee changes" do
include_examples "change combos",
Consts::APP_COFFEE_PATH,
Consts::COFFEE_SPEC_PATH
context "bundles" do
include_examples "does not comment",
[{path: Consts::APP_COFFEE_BUNDLE_PATH, status: "added"}]
end
context "with jsx spec changes" do
include_examples "change combos",
Consts::APP_COFFEE_PATH,
Consts::JSX_SPEC_PATH
end
end
end

View File

@ -0,0 +1,11 @@
require 'spec_helper'
require_relative "../shared_constants"
require_relative "../shared_linter_examples"
describe TatlTael::Linters::Simple::JsxSpecsLinter do
let(:config) { TatlTael::Linters.config_for_linter(described_class) }
include_examples "change combos",
Consts::APP_JSX_PATH,
Consts::JSX_SPEC_PATH
end

View File

@ -0,0 +1,23 @@
require 'spec_helper'
require_relative "../shared_constants"
require_relative "../shared_linter_examples"
describe TatlTael::Linters::Simple::NewErbLinter do
let(:config) { TatlTael::Linters.config_for_linter(described_class) }
context "app views erb additions exist" do
include_examples "comments", [{path: Consts::APP_ERB_PATH, status: "added"}]
end
context "other erb additions exist" do
include_examples "does not comment", [{path: Consts::OTHER_ERB_PATH, status: "added"}]
end
context "erb non additions exist" do
include_examples "does not comment", [{path: Consts::APP_ERB_PATH, status: "modified"}]
end
context "no erb changes exist" do
include_examples "does not comment", [{path: Consts::PUBLIC_VENDOR_JS_PATH, status: "added"}]
end
end

View File

@ -0,0 +1,26 @@
require 'spec_helper'
require_relative "../shared_constants"
require_relative "../shared_linter_examples"
describe TatlTael::Linters::Simple::PublicJsSpecsLinter do
let(:config) { TatlTael::Linters.config_for_linter(described_class) }
include_examples "change combos",
Consts::PUBLIC_JS_PATH,
Consts::PUBLIC_JS_SPEC_PATH
context "in excluded public sub dirs" do
context "bower" do
include_examples "does not comment",
[{path: Consts::PUBLIC_BOWER_JS_PATH, status: "added"}]
end
context "mediaelement" do
include_examples "does not comment",
[{path: Consts::PUBLIC_ME_JS_PATH, status: "added"}]
end
context "vendor" do
include_examples "does not comment",
[{path: Consts::PUBLIC_VENDOR_JS_PATH, status: "added"}]
end
end
end

View File

@ -0,0 +1,143 @@
require 'spec_helper'
require_relative "./shared_constants"
describe TatlTael::Linters::SimpleLinter do
let(:config_message) { "blarghishfarbin" }
let(:config) do
{
"Severity" => "warn",
"Message" => config_message,
"Precondition" => {
"Statuses" => %w[added deleted],
"Include" => ["**/yarg/**"],
"Whitelist" => ["**/yarg/blargh/**"],
}
}
end
let(:pretty_config) { TatlTael::Linters.underscore_and_symbolize_keys(config) }
let(:changes) { double }
let(:simple_linter) { described_class.new(config: pretty_config, changes: changes) }
describe "#run" do
context "precondition NOT met" do
before :each do
allow(simple_linter).to receive(:precondition_met?).and_return(false)
end
it "returns nothing" do
expect(simple_linter.run).to be_nil
end
end
context "precondition met" do
before :each do
allow(simple_linter).to receive(:precondition_met?).and_return(true)
end
context "requirement met" do
before :each do
allow(simple_linter).to receive(:requirement_met?).and_return(true)
end
it "returns nothing" do
expect(simple_linter.run).to be_nil
end
end
context "requirement NOT met" do
let(:comment) do
{
severity: "warn",
cover_message: true,
message: config_message
}
end
before :each do
allow(simple_linter).to receive(:requirement_met?).and_return(false)
end
it "returns comment" do
expect(simple_linter.run).to match(hash_including(comment))
end
end
end
end
describe "#precondition_met?" do
context "changes exist for the precondition query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(pretty_config[:precondition])
.and_return(true)
end
it "returns true" do
expect(simple_linter.precondition_met?).to be_truthy
end
end
context "changes DO NOT exist for the precondition query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(pretty_config[:precondition])
.and_return(false)
end
it "returns false" do
expect(simple_linter.precondition_met?).to be_falsey
end
end
end
describe "#requirement_met?" do
context "no requirement query in config" do
it "returns false" do
expect(simple_linter.requirement_met?).to be_falsey
end
end
context "requirement query exists in config" do
let(:requirement) do
{
"Statuses" => %w[modified deleted],
"Include" => ["**/reeee/**"],
"Whitelist" => ["**/reeee/blarghy/**"],
}
end
let(:config_with_requirement) { config.merge("Requirement" => requirement) }
let(:config_with_pretty_requirement) do
TatlTael::Linters.underscore_and_symbolize_keys(config_with_requirement)
end
let(:simple_linter) do
described_class.new(config: config_with_pretty_requirement,
changes: changes)
end
context "changes exist for the requirement query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(config_with_pretty_requirement[:requirement])
.and_return(true)
end
it "returns true" do
expect(simple_linter.requirement_met?).to be_truthy
end
end
context "changes DO NOT exist for the requirement query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(config_with_pretty_requirement[:requirement])
.and_return(false)
end
it "returns false" do
expect(simple_linter.requirement_met?).to be_falsey
end
end
end
end
end

View File

@ -4,7 +4,7 @@ describe TatlTael::Linters do
describe TatlTael::Linters::BaseLinter do
describe ".inherited" do
context "not a simple linter" do
class FooLinter < TatlTael::Linters::BaseLinter;
class FooLinter < TatlTael::Linters::BaseLinter
end
it "saves the subclass" do
@ -22,11 +22,12 @@ describe TatlTael::Linters do
describe "#changes_matching" do
Change = Struct.new(:status, :path)
let(:base_linter) { TatlTael::Linters::BaseLinter.new(changes) }
let(:config) { {} }
let(:base_linter) { TatlTael::Linters::BaseLinter.new(config: config, changes: changes) }
before :each do
allow(base_linter).to receive(:changes)
.and_return(changes)
.and_return(changes)
end
context "filtering by statuses" do
@ -52,7 +53,7 @@ describe TatlTael::Linters do
end
end
context "filtering by include_regexes" do
context "filtering by includes" do
let(:added_change_path) { "path/to/foo" }
let(:added_change) { Change.new("added", added_change_path) }
let(:modified_change_path) { "path/to/mod" }
@ -64,16 +65,16 @@ describe TatlTael::Linters do
expect(base_linter.changes_matching).to match(changes)
end
context "include_regexes exist" do
let(:query) { {include_regexes: [/zoo/, /foo/, /bar/]} }
context "includes exist" do
let(:query) { {include: ["**/zoo", "**/foo", "**/bar"]} }
it "returns the changes that match any of the include_regexes" do
it "returns the changes that match any of the includes" do
expect(base_linter.changes_matching(query)).to match([added_change])
end
end
end
context "filtering by exclude_regexes" do
context "filtering by whitelist" do
let(:added_change_path) { "path/to/foo" }
let(:added_change) { Change.new("added", added_change_path) }
let(:modified_change_path) { "path/to/mod" }
@ -86,9 +87,9 @@ describe TatlTael::Linters do
end
context "include_regexes exist" do
let(:query) { {exclude_regexes: [/zoo/, /foo/, /bar/]} }
let(:query) { {whitelist: ["**/zoo", "**/foo", "**/bar"]} }
it "returns the changes that don't match any of the exclude_regexes" do
it "returns the changes that don't match any of the whitelists" do
expect(base_linter.changes_matching(query)).to match([modified_change])
end
end
@ -103,12 +104,13 @@ describe TatlTael::Linters do
exclude_regexes: [/^public/]
}
end
let(:base_linter) { TatlTael::Linters::BaseLinter.new(changes) }
let(:config) { {} }
let(:base_linter) { TatlTael::Linters::BaseLinter.new(config: config, changes: changes) }
before :each do
allow(base_linter).to receive(:changes_matching)
.with(hash_including(query))
.and_return(changes)
.with(hash_including(query))
.and_return(changes)
end
@ -138,21 +140,6 @@ describe TatlTael::Linters do
let(:changes) { double }
describe ".comments" do
let(:simple_comments) { [["a", "b", nil, "c"]] }
let(:reg_comments) { nil }
it "collects simple and regular linter comments" do
expect(TatlTael::Linters::SimpleLinter).to receive(:comments)
.with(changes)
.and_return(simple_comments)
expect(linters).to receive(:run_linters)
.with(changes)
.and_return(reg_comments)
expect(linters.comments(changes)).to match(%w[a b c])
end
end
describe ".run_linters" do
class BarLinter < TatlTael::Linters::BaseLinter
def run
[[], [nil], "1"]
@ -166,9 +153,9 @@ describe TatlTael::Linters do
let(:saved_linters) { [BarLinter, ZooLinter] }
it "collects regular linter comments" do
expect(TatlTael::Linters).to receive(:linters).and_return(saved_linters)
expect(linters.run_linters(changes)).to match(%w[1 2 3])
it "collects linter comments" do
expect(linters).to receive(:linters).and_return(saved_linters)
expect(linters.comments(changes)).to match(%w[1 2 3])
end
end
end

View File

@ -1,270 +0,0 @@
require 'spec_helper'
require_relative "./shared_constants"
describe TatlTael::Linters::SimpleLinter do
let(:config_message) { "blarghishfarbin" }
let(:config) do
{
message: config_message,
precondition: double
}
end
let(:changes) { double }
let(:simple_linter) { described_class.new(config: config, changes: changes) }
describe "#run" do
context "precondition NOT met" do
before :each do
allow(simple_linter).to receive(:precondition_met?).and_return(false)
end
it "returns nothing" do
expect(simple_linter.run).to be_nil
end
end
context "precondition met" do
before :each do
allow(simple_linter).to receive(:precondition_met?).and_return(true)
end
context "requirement met" do
before :each do
allow(simple_linter).to receive(:requirement_met?).and_return(true)
end
it "returns nothing" do
expect(simple_linter.run).to be_nil
end
end
context "requirement NOT met" do
let(:comment) do
{
severity: "warn",
cover_message: true,
message: config_message
}
end
before :each do
allow(simple_linter).to receive(:requirement_met?).and_return(false)
end
it "returns comment" do
expect(simple_linter.run).to match(hash_including(comment))
end
end
end
end
describe "#precondition_met?" do
context "changes exist for the precondition query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(config[:precondition])
.and_return(true)
end
it "returns true" do
expect(simple_linter.precondition_met?).to be_truthy
end
end
context "changes DO NOT exist for the precondition query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(config[:precondition])
.and_return(false)
end
it "returns false" do
expect(simple_linter.precondition_met?).to be_falsey
end
end
end
describe "#requirement_met?" do
context "no requirement query in config" do
it "returns false" do
expect(simple_linter.requirement_met?).to be_falsey
end
end
context "requirement query exists in config" do
let(:requirement) { double("requirement") }
let(:config_with_requirement) { config.merge(requirement: requirement) }
let(:simple_linter) do
described_class.new(config: config_with_requirement,
changes: changes)
end
context "changes exist for the requirement query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(requirement)
.and_return(true)
end
it "returns true" do
expect(simple_linter.requirement_met?).to be_truthy
end
end
context "changes DO NOT exist for the requirement query" do
before :each do
allow(simple_linter).to receive(:changes_exist?)
.with(requirement)
.and_return(false)
end
it "returns false" do
expect(simple_linter.requirement_met?).to be_falsey
end
end
end
end
describe ".comments" do
let(:simple_linter_class) { described_class }
let(:configs) do
simple_linter_class.configs.select { |config| config[:name] == linter }
end
let(:config) { configs.first }
let(:linter_comment) do
{
severity: "warn",
cover_message: true,
message: config[:message]
}
end
before(:each) do
allow(simple_linter_class).to receive(:configs).and_return(configs)
end
shared_examples "comments" do |raw_changes|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter_config) { { config: config, changes: changes } }
let(:simple_linter) { described_class.new(linter_config) }
it "comments" do
allow(simple_linter_class).to receive(:new)
.with(hash_including(linter_config))
.and_return(simple_linter)
result = simple_linter_class.comments(changes)
expect(result.size).to eq(1)
expect(result.first).to match(hash_including(linter_comment))
end
end
shared_examples "does not comment" do |raw_changes|
let(:changes) { raw_changes.map { |c| double(c) } }
let(:linter_config) { {config: config, changes: changes} }
let(:simple_linter) { described_class.new(linter_config) }
it "does not comment" do
allow(simple_linter_class).to receive(:new)
.with(hash_including(linter_config))
.and_return(simple_linter)
expect(simple_linter_class.comments(changes)).to be_empty
end
end
shared_examples "change combos" do |change_path, spec_path|
context "not deletion" do
context "no spec changes" do
include_examples "comments",
[{ path: change_path, status: "added" }]
end
context "has spec non deletions" do
include_examples "does not comment",
[{ path: change_path, status: "modified" },
{ path: spec_path, status: "added" }]
end
context "has spec deletions" do
include_examples "comments",
[{ path: change_path, status: "added" },
{ path: spec_path, status: "deleted" }]
end
end
context "deletion" do
include_examples "does not comment",
[{ path: change_path, status: "deleted" }]
end
end
describe "CoffeeSpecsLinter" do
let(:linter) { "CoffeeSpecsLinter" }
context "coffee changes" do
include_examples "change combos",
Consts::APP_COFFEE_PATH,
Consts::COFFEE_SPEC_PATH
context "bundles" do
include_examples "does not comment",
[{path: Consts::APP_COFFEE_BUNDLE_PATH, status: "added"}]
end
context "with jsx spec changes" do
include_examples "change combos",
Consts::APP_COFFEE_PATH,
Consts::JSX_SPEC_PATH
end
end
end
describe "PublicJsSpecsLinter" do
let(:linter) { "PublicJsSpecsLinter" }
include_examples "change combos",
Consts::PUBLIC_JS_PATH,
Consts::PUBLIC_JS_SPEC_PATH
context "in excluded public sub dirs" do
context "bower" do
include_examples "does not comment",
[{ path: Consts::PUBLIC_BOWER_JS_PATH, status: "added" }]
end
context "mediaelement" do
include_examples "does not comment",
[{ path: Consts::PUBLIC_ME_JS_PATH, status: "added" }]
end
context "vendor" do
include_examples "does not comment",
[{ path: Consts::PUBLIC_VENDOR_JS_PATH, status: "added" }]
end
end
end
describe "JsxSpecsLinter" do
let(:linter) { "JsxSpecsLinter" }
include_examples "change combos",
Consts::APP_JSX_PATH,
Consts::JSX_SPEC_PATH
end
describe "NewErbLinter" do
let(:linter) { "NewErbLinter" }
context "app views erb additions exist" do
include_examples "comments", [{path: Consts::APP_ERB_PATH, status: "added"}]
end
context "other erb additions exist" do
include_examples "does not comment", [{path: Consts::OTHER_ERB_PATH, status: "added"}]
end
context "erb non additions exist" do
include_examples "does not comment", [{path: Consts::APP_ERB_PATH, status: "modified"}]
end
context "no erb changes exist" do
include_examples "does not comment", [{path: Consts::PUBLIC_VENDOR_JS_PATH, status: "added"}]
end
end
end
end