From fbe1163faa3fea58eb9640746c208fa7f18c6778 Mon Sep 17 00:00:00 2001 From: Ryan Shaw Date: Mon, 26 Aug 2019 21:07:44 -0600 Subject: [PATCH] update JavaScript tatl_tael lint messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this is to reflect the current state of what we consider “best practices” test plan: * if you make a commit that creates a new qUnit spec file, gergich should complain to you Change-Id: I502fe1261880c82f83f6d813b3c99c4a1f00b0f1 Reviewed-on: https://gerrit.instructure.com/206878 Tested-by: Jenkins Reviewed-by: Clay Diffrient QA-Review: Ryan Shaw Product-Review: Ryan Shaw --- gems/tatl_tael/config/default.yml | 44 +++++++++++++------ .../tatl_tael/linters/simple/coffee_linter.rb | 10 +++++ .../linters/simple/new_qunit_linter.rb | 10 +++++ .../simple/coffee_specs_linter_spec.rb | 5 --- 4 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 gems/tatl_tael/lib/tatl_tael/linters/simple/coffee_linter.rb create mode 100644 gems/tatl_tael/lib/tatl_tael/linters/simple/new_qunit_linter.rb diff --git a/gems/tatl_tael/config/default.yml b/gems/tatl_tael/config/default.yml index 153bb573a39..3ca40fb6e21 100644 --- a/gems/tatl_tael/config/default.yml +++ b/gems/tatl_tael/config/default.yml @@ -153,20 +153,29 @@ SeleniumSpecsLinter: 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/CoffeeLinter: + Severity: "warn" + Precondition: + Include: + - "**/**.coffee" + Message: Your commit modifies CoffeeScript files. Don't do that. + CoffeeScript is deprecated in canvas and we eventually want to get rid of all of it. + If you are going to modify any code that is currently CoffeScript, + first decaffeinate (https://decaffeinate-project.org/repl/) it to .js in one commit, + and then make the changes in that .js file in a second commit. + Simple/CoffeeSpecsLinter: Severity: "warn" Precondition: Include: - "app/coffeescripts/**.coffee" - Whitelist: - - "app/coffeescripts/bundles/**" Requirement: Include: - "app/jsx/**/__tests__/**" - - "spec/coffeescripts/**.coffee" - - "spec/coffeescripts/jsx/**" - - "spec/javascripts/jsx/**" - Message: Your commit includes coffee changes, but does not include coffee or jsx + - "app/coffeescripts/**/__tests__/**" + - "spec/coffeescripts/**" + - "spec/javascripts/**" + Message: Your commit includes coffee changes, but does not include Jest specs. Please add some to verify your changes. Simple/JsxSpecsLinter: @@ -177,9 +186,8 @@ Simple/JsxSpecsLinter: Requirement: Include: - "app/jsx/**/__tests__/**" - - "spec/coffeescripts/jsx/**" - "spec/javascripts/jsx/**" - Message: Your commit includes coffee changes, but does not include coffee or jsx + Message: Your commit includes JavaScript changes, but does not include Jest specs. Please add some to verify your changes. Simple/PublicJsSpecsLinter: @@ -190,16 +198,15 @@ Simple/PublicJsSpecsLinter: Whitelist: - "**/bower/**" - "**/mediaelement/**" - - "**/shims/**" - "**/vendor/**" - "**/symlink_to_node_modules/**" Requirement: Include: - "app/jsx/**/__tests__/**" - - "spec/coffeescripts/**" + - "public/javascripts/**/__tests__/**" - "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 + Jest specs. Please add some to verify your changes. Even $.fn.crazyMethods can and should be tested (and not via selenium). Simple/NewErbLinter: @@ -210,8 +217,8 @@ Simple/NewErbLinter: 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: + Your commit includes new ERB files but you may be able to accomplish + everything you need by doing something like this in your controller instead: @page_title = t('Your Page Title') add_body_class 'whatever-classes you-want-to-add-to-body' @@ -219,3 +226,14 @@ Simple/NewErbLinter: css_bundle :any_css_bundles_you_want js_env({whatever: 'you need to put in window.ENV'}) render :html => "".html_safe, :layout => true + +Simple/NewQunitLinter: + Severity: "error" + Precondition: + Include: + - "spec/coffeescripts/**" + - "spec/javascripts/**" + Statuses: + - added + Message: |- + Your commit includes new qUnit/karma files. Don't do that. All new tests should be in Jest. diff --git a/gems/tatl_tael/lib/tatl_tael/linters/simple/coffee_linter.rb b/gems/tatl_tael/lib/tatl_tael/linters/simple/coffee_linter.rb new file mode 100644 index 00000000000..754110f4cba --- /dev/null +++ b/gems/tatl_tael/lib/tatl_tael/linters/simple/coffee_linter.rb @@ -0,0 +1,10 @@ +require_relative "../simple_linter" + +module TatlTael + module Linters + module Simple + class CoffeeLinter < SimpleLinter + end + end + end +end diff --git a/gems/tatl_tael/lib/tatl_tael/linters/simple/new_qunit_linter.rb b/gems/tatl_tael/lib/tatl_tael/linters/simple/new_qunit_linter.rb new file mode 100644 index 00000000000..8198cbce963 --- /dev/null +++ b/gems/tatl_tael/lib/tatl_tael/linters/simple/new_qunit_linter.rb @@ -0,0 +1,10 @@ +require_relative "../simple_linter" + +module TatlTael + module Linters + module Simple + class NewQunitLinter < SimpleLinter + end + end + end +end diff --git a/gems/tatl_tael/spec/lib/tatl_tael/linters/simple/coffee_specs_linter_spec.rb b/gems/tatl_tael/spec/lib/tatl_tael/linters/simple/coffee_specs_linter_spec.rb index 87bc7fa6ba0..d4ee621cff9 100644 --- a/gems/tatl_tael/spec/lib/tatl_tael/linters/simple/coffee_specs_linter_spec.rb +++ b/gems/tatl_tael/spec/lib/tatl_tael/linters/simple/coffee_specs_linter_spec.rb @@ -10,11 +10,6 @@ describe TatlTael::Linters::Simple::CoffeeSpecsLinter do 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,