Skip to content

Commit 216ab21

Browse files
author
Landon Wilkins
committed
improve tatl_tael for public/javascripts changes, fixes SD-1196
test plan: * test commit Change-Id: Iba8695913f05876dc076e2bb7b1e6d0584ad0710 Reviewed-on: https://gerrit.instructure.com/80944 Reviewed-by: Jon Jensen <jon@instructure.com> Product-Review: Jon Jensen <jon@instructure.com> QA-Review: Jon Jensen <jon@instructure.com> Tested-by: Jenkins
1 parent be337ee commit 216ab21

3 files changed

Lines changed: 81 additions & 32 deletions

File tree

‎gems/tatl_tael/lib/tatl_tael/linter.rb‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,17 @@ def ensure_jsx_specs
2626
yield if needs_jsx_specs? && !jsx_specs?
2727
end
2828

29+
def ensure_public_js_specs
30+
yield if needs_public_js_specs? && !public_js_specs
31+
end
32+
2933
def ensure_ruby_specs
3034
yield if needs_ruby_specs? && !ruby_specs?
3135
end
3236

3337
def ensure_no_unnecessary_selenium_specs
3438
yield if selenium_specs? && (
39+
needs_public_js_specs? && !public_js_specs ||
3540
needs_coffee_specs? && !coffee_specs? ||
3641
needs_jsx_specs? && !jsx_specs? ||
3742
needs_ruby_specs? && !ruby_specs?
@@ -50,13 +55,31 @@ def new_erb?
5055
end
5156
end
5257

53-
NEED_COFFEE_SPECS_REGEX = /(app\/coffeescripts\/.*\.coffee|public\/javascripts\/.*.js)$/
54-
EXCLUDED_SUB_DIRS_REGEX = /(bundles|bower|mediaelement|shims|vendor)\//
58+
NEED_SPEC_PUBLIC_JS_REGEX = /public\/javascripts\/.*.js$/
59+
EXCLUDED_PUBLIC_SUB_DIRS_REGEX = /(bower|mediaelement|shims|vendor)\//
60+
def needs_public_js_specs?
61+
changes.any? do |change|
62+
!change.deleted? &&
63+
change.path =~ NEED_SPEC_PUBLIC_JS_REGEX &&
64+
change.path !~ EXCLUDED_PUBLIC_SUB_DIRS_REGEX
65+
end
66+
end
67+
68+
PUBLIC_JS_SPEC_REGEX = /spec\/(coffeescripts|javascripts)\//
69+
def public_js_specs
70+
changes.any? do |change|
71+
!change.deleted? &&
72+
change.path =~ PUBLIC_JS_SPEC_REGEX
73+
end
74+
end
75+
76+
NEED_COFFEE_SPECS_REGEX = /app\/coffeescripts\/.*\.coffee$/
77+
EXCLUDED_COFFEE_SUB_DIRS_REGEX = /bundles\//
5578
def needs_coffee_specs?
5679
changes.any? do |change|
5780
!change.deleted? &&
5881
change.path =~ NEED_COFFEE_SPECS_REGEX &&
59-
change.path !~ EXCLUDED_SUB_DIRS_REGEX
82+
change.path !~ EXCLUDED_COFFEE_SUB_DIRS_REGEX
6083
end
6184
end
6285

@@ -76,11 +99,11 @@ def needs_jsx_specs?
7699
end
77100
end
78101

79-
JSX_SPEC_REGEX = /spec\/jsx\//
102+
JSX_SPEC_REGEX = /spec\/(coffeescripts|javascripts)\/jsx\//
80103
def jsx_specs?
81104
changes.any? do |change|
82105
!change.deleted? &&
83-
change.path =~ COFFEE_SPEC_REGEX
106+
change.path =~ JSX_SPEC_REGEX
84107
end
85108
end
86109

‎gems/tatl_tael/spec/linter_spec.rb‎

Lines changed: 45 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,16 +47,17 @@
4747
COFFEE_SPEC_PATH = "spec/coffeescripts/calendar/CalendarSpec.coffee"
4848

4949
APP_JSX_PATH = "app/jsx/dashboard_card/DashboardCardAction.jsx"
50-
JSX_SPEC_PATH = "spec/coffeescripts/jsx/dashboard_card/DashboardCardActionSpec.coffee"
50+
JSX_SPEC_PATH = "spec/javascripts/jsx/dashboard_card/DashboardCardActionSpec.coffee"
5151

5252
APP_RB_PATH = "app/controllers/accounts_controller.rb"
5353
APP_RB_SPEC_PATH = "spec/controllers/accounts_controller_spec.rb"
5454
LIB_RB_PATH = "lib/reporting/counts_report.rb"
5555
LIB_RB_SPEC_PATH = "spec/lib/reporting/counts_report_spec.rb"
5656

5757
APP_ERB_PATH = "app/views/announcements/index.html.erb"
58-
PUBLIC_HTML_PATH = "public/partials/_license_help.html"
59-
PUBLIC_JS_PATH = "public/javascripts/account_settings.js"
58+
PUBLIC_JS_PATH = "public/javascripts/eportfolios/eportfolio_section.js"
59+
PUBLIC_JS_SPEC_PATH = "spec/javascripts/jsx/eportfolios/eportfolioSectionSpec.jsx"
60+
6061
PUBLIC_BOWER_JS_PATH = "public/javascripts/bower/axios/dist/axios.amd.js"
6162
PUBLIC_ME_JS_PATH = "public/javascripts/mediaelement/mep-feature-speed-instructure.js"
6263
PUBLIC_VENDOR_JS_PATH = "public/javascripts/vendor/bootstrap/bootstrap-dropdown.js"
@@ -79,29 +80,29 @@
7980
:ensure_coffee_specs
8081
end
8182
end
83+
end
8284

83-
context "js changes" do
84-
include_examples "change combos",
85-
PUBLIC_JS_PATH,
86-
COFFEE_SPEC_PATH,
87-
:ensure_coffee_specs
85+
describe "#ensure_public_js_specs" do
86+
include_examples "change combos",
87+
PUBLIC_JS_PATH,
88+
PUBLIC_JS_SPEC_PATH,
89+
:ensure_public_js_specs
8890

89-
context "in excluded public sub dirs" do
90-
context "bower" do
91-
include_examples "does not yield",
92-
[{ path: PUBLIC_BOWER_JS_PATH, deleted?: false }],
93-
:ensure_coffee_specs
94-
end
95-
context "mediaelement" do
96-
include_examples "does not yield",
97-
[{ path: PUBLIC_ME_JS_PATH, deleted?: false }],
98-
:ensure_coffee_specs
99-
end
100-
context "vendor" do
101-
include_examples "does not yield",
102-
[{ path: PUBLIC_VENDOR_JS_PATH, deleted?: false }],
103-
:ensure_coffee_specs
104-
end
91+
context "in excluded public sub dirs" do
92+
context "bower" do
93+
include_examples "does not yield",
94+
[{ path: PUBLIC_BOWER_JS_PATH, deleted?: false }],
95+
:ensure_public_js_specs
96+
end
97+
context "mediaelement" do
98+
include_examples "does not yield",
99+
[{ path: PUBLIC_ME_JS_PATH, deleted?: false }],
100+
:ensure_public_js_specs
101+
end
102+
context "vendor" do
103+
include_examples "does not yield",
104+
[{ path: PUBLIC_VENDOR_JS_PATH, deleted?: false }],
105+
:ensure_public_js_specs
105106
end
106107
end
107108
end
@@ -131,6 +132,23 @@
131132

132133
describe "#ensure_no_unnecessary_selenium_specs" do
133134
context "has selenium specs" do
135+
context "needs public js specs" do
136+
context "has no public js specs" do
137+
include_examples "yields",
138+
[{ path: SELENIUM_SPEC_PATH, deleted?: false },
139+
{ path: PUBLIC_JS_PATH, deleted?: false }],
140+
:ensure_no_unnecessary_selenium_specs
141+
end
142+
143+
context "has public js specs" do
144+
include_examples "does not yield",
145+
[{ path: SELENIUM_SPEC_PATH, deleted?: false },
146+
{ path: PUBLIC_JS_PATH, deleted?: false },
147+
{ path: PUBLIC_JS_SPEC_PATH, deleted?: false }],
148+
:ensure_no_unnecessary_selenium_specs
149+
end
150+
end
151+
134152
context "needs coffee specs" do
135153
context "has no coffee specs" do
136154
include_examples "yields",
@@ -192,23 +210,23 @@
192210

193211
describe "#ban_new_erb" do
194212
context "erb additions exist" do
195-
let(:changes) { [double(path: "yarg.erb", added?: true)] }
213+
let(:changes) { [double(path: APP_ERB_PATH, added?: true)] }
196214

197215
it "yields" do
198216
expect { |b| subject.ban_new_erb(&b) }.to yield_with_no_args
199217
end
200218
end
201219

202220
context "erb non additions exist" do
203-
let(:changes) { [double(path: "yarg.erb", added?: false)] }
221+
let(:changes) { [double(path: APP_ERB_PATH, added?: false)] }
204222

205223
it "does not yield" do
206224
expect { |b| subject.ban_new_erb(&b) }.not_to yield_with_no_args
207225
end
208226
end
209227

210228
context "no erb changes exist" do
211-
let(:changes) { [double(path: "yarg.js", added?: true)] }
229+
let(:changes) { [double(path: PUBLIC_VENDOR_JS_PATH, added?: true)] }
212230

213231
it "does not yield" do
214232
expect { |b| subject.ban_new_erb(&b) }.not_to yield_with_no_args

‎script/tatl_tael‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,14 @@ linter.ban_new_erb do
4646
" All new UI should be built in React on top of documented APIs"
4747
end
4848

49+
linter.ensure_public_js_specs do
50+
errors << "Your commit includes changes to public/javascripts,"\
51+
" but does not include specs (coffee or jsx)."\
52+
" Please add some to verify your changes."\
53+
" Even $.fn.crazyMethods can and should be tested"\
54+
" (and not via selenium)."
55+
end
56+
4957
linter.ensure_coffee_specs do
5058
errors << "Your commit includes coffee changes,"\
5159
" but does not include coffee specs."\

0 commit comments

Comments
 (0)