closes#1565
test plan: rubocop still works
Change-Id: I77752b64224e176e5a4aff6d470cd85354b05474
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/225005
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
some hot takes:
- 4.times.map is more readible to Array.new(4).map and has no perf gain
in small cases which is the overwhelming use case for specs. This
also has no perf impact for app code since it's excluded for specs only.
- change { AnonymousOrModerationEvent.count } is more readible than
change(AnonymousOrModerationEvent, :count) because the former is the
exact code you would see in your app code. This is an RSpec only cop
so it has no impact on app code.
Change-Id: I6f70b6a9f69a3375cf6f8db27eadd810f25361ca
Reviewed-on: https://gerrit.instructure.com/162540
Tested-by: Jenkins
Reviewed-by: Spencer Olson <solson@instructure.com>
Reviewed-by: Cameron Matheson <cameron@instructure.com>
Product-Review: Derek Bender <djbender@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
We're already ignoring it in RSpec and Selenium tests. And Pact
tests inherently have a lot of lines.
Change-Id: Ia976224a03426864beb04d885011e982d4c2a60d
Reviewed-on: https://gerrit.instructure.com/156583
Reviewed-by: Michael Hargiss <mhargiss@instructure.com>
Tested-by: Jenkins
Product-Review: Tucker McKnight <tmcknight@instructure.com>
QA-Review: Tucker McKnight <tmcknight@instructure.com>
to be the one we used most often
Change-Id: I0ed597588f48d2d3f57111c5a409b1304ebab666
Reviewed-on: https://gerrit.instructure.com/143446
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Rob Orton <rob@instructure.com>
QA-Review: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
yes, some of them are kind of sad/bad. but the signal to noise with them
on is so bad that we end up ignoring far more useful advice because
we ignore gergich completely
Change-Id: Ie62b9e4c6459440c7c6cfaa29bae8271be639d77
Reviewed-on: https://gerrit.instructure.com/141069
Reviewed-by: Rob Orton <rob@instructure.com>
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
also fix a typo in .eslintignore caused by cae5d84950
motivation:
https://github.com/bbatsov/rubocop/issues/4222#issuecomment-290655562
this is a deliberate idiom in rspec, so let's update .robocop.yml to
match rubocop's own config
closes: GRADE-841
test plan: specs pass
Change-Id: I534eabe48fb227b6fa7b425502e645f22b1a51f3
Reviewed-on: https://gerrit.instructure.com/140424
Tested-by: Jenkins
Reviewed-by: Keith T. Garner <kgarner@instructure.com>
Product-Review: Derek Bender <djbender@instructure.com>
QA-Review: Derek Bender <djbender@instructure.com>
rubocop changed the namespace on some items. This updates to the new
names so that it won't toss out warnings that things have moved.
Change-Id: Ie9003672a10adebab983b1b91393cc83a207b3c3
Reviewed-on: https://gerrit.instructure.com/136688
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
Product-Review: Keith T. Garner <kgarner@instructure.com>
QA-Review: Keith T. Garner <kgarner@instructure.com>
Change-Id: Ic510fa4a6b34227cf8c6b01bdeea98d9325d1a0d
Reviewed-on: https://gerrit.instructure.com/136139
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
beginning of line can be dangerous when you copy/paste into irb,
because it's not greedy, so will execute whatever looks like
the first complete statement. this may be an AR query that hasn't
been very restricted yet, accidentally destroying the database
Change-Id: I878e9e364b84ae57d79ea3185d3bddf3e6d56d8a
Reviewed-on: https://gerrit.instructure.com/133054
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
Change-Id: I83c54de0e4ec48142c738568d14e54db5766dbbb
Reviewed-on: https://gerrit.instructure.com/131136
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Change-Id: I1aef2dee2089bb61bf220bd8a3640b3c90772188
Reviewed-on: https://gerrit.instructure.com/131135
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
also prevent them in the future. our test-queue fork currently has a bug
where only the last duplicate spec will run, but this is a good idea anyway
most of these are just rewordings, but in some cases delete redundant/
identical specs
test plan: specs
Change-Id: I8f51be55e750cf87ffa8150822f4b80dc99e1eb4
Reviewed-on: https://gerrit.instructure.com/103993
Tested-by: Jenkins
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Landon Wilkins <lwilkins@instructure.com>
QA-Review: Landon Wilkins <lwilkins@instructure.com>
Change-Id: I380b57bc6ee4e7b8e606e5148fb0468031ccee57
Reviewed-on: https://gerrit.instructure.com/101538
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
* before(:each) is nice since it's often right next to before(:once)
* RMessageSpies isn't a thing
* don't complain about big blocks (e.g `describe`)
Change-Id: I296433cf2478ac3a304a2e20a1a3cd5de6515678
Reviewed-on: https://gerrit.instructure.com/101502
Tested-by: Jenkins
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
closes CNVS-34657
test plan: rubocop should still work
Change-Id: Ibdd340dd7ba516639ee5b7225449422b324bd33a
Reviewed-on: https://gerrit.instructure.com/100831
Tested-by: Jenkins
Reviewed-by: Tyler Pickett <tpickett@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
This fixes the rule setting the limit for nested example groups in
RSpec. Its current setting is invalid and does not work.
test plan:
* verify Jenkins passes
Change-Id: If6e03df1d9fd66bc5aacf1f78e0918086c6c73de
Reviewed-on: https://gerrit.instructure.com/100780
Reviewed-by: Simon Williams <simon@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Neander <jneander@instructure.com>
Product-Review: Jeremy Neander <jneander@instructure.com>
Change-Id: Ib916de654fa303c2c26d17ca23c6e113d212fa5f
Reviewed-on: https://gerrit.instructure.com/100204
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
This disables the HttpPositionalArguments rubocop rule, which is invalid
for versions of Ruby prior to Ruby 5. This also sets the limit for
nested example groups in RSpec to 3, which is a more reasonable limit
than the default of 2.
test plan:
* verify Jenkins passes
Change-Id: I8f2badb369d6932923aa4c096c9b9014be19805a
Reviewed-on: https://gerrit.instructure.com/100198
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: Jeremy Neander <jneander@instructure.com>
Product-Review: Jeremy Neander <jneander@instructure.com>
Change-Id: Ice28cad151efa8f58e45b88eb3b11ac248d07c7b
Reviewed-on: https://gerrit.instructure.com/100104
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
Change-Id: I185cc5dbfaafe64f74a511aac1ab9b94bae38d63
Reviewed-on: https://gerrit.instructure.com/98776
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
This cop encourages explit require_dependency calls for ambiguous nested
constants in specs. What does that mean? Consider:
module Analytics
describe Assignments do
Depending on what has been required and/or autoloaded, `Assignments` could
either resolve to `Analytics::Assignments`, or just the top-level
`Assignments`. This is a cause of flickering failures and test-queue woes.
This example should either have an explicit `require_dependency` call, or
get rid of the module and just do `describe Analytics::Assignments``
Correct all existing ones in the codebase, except for a few
ActiveRecord-related ones (the auto-correct isn't quite perfect, i.e. it
assumes the file path will be `const_name.underscore`, which is no bueno
for things like ActiveRecord::ConnectionAdapters::PostgreSQLAdapter)
Test plan:
n/a, specs
Change-Id: Ic24bf3e0f547ca11c46887d4af92804da091912a
Reviewed-on: https://gerrit.instructure.com/98752
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
note: Astrolabe::Node is now just Rubocop::Node
closes CNVS-34077
test plan: rubocop should still lint correctly
Change-Id: Id5feb1700ad78cc0b34c6cb90d2a75320b210138
Reviewed-on: https://gerrit.instructure.com/98512
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Simon Williams <simon@instructure.com>
This reverts commit 2073553507.
This reverts commit 56381d06b4.
rubocop upgraded today, and we tried a couple times to fix it, but since
we only run rubocop when ruby files change, the fixes themselves passed
the build but didn't fully fix the problem. for now, until we can
complete an upgrade, just pin the known working version of rubocop.
Change-Id: Ib7587e5ff9f3c00817f920506208a0e20444d379
Reviewed-on: https://gerrit.instructure.com/70356
Reviewed-by: Jon Jensen <jon@instructure.com>
Tested-by: Jenkins
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
rubocop wasn't version pinned, so jenkins got a later version,
and the aux build started failing because of a config difference.
this fixes the config and pins the version of rubocop
Change-Id: I5d94b779a8d29dac66ad0e121737816459c62ba3
Reviewed-on: https://gerrit.instructure.com/70337
Reviewed-by: Jon Jensen <jon@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Ethan Vizitei <evizitei@instructure.com>
Style/ParallelAssignment has the wrong namespace - should be Performance
Change-Id: Ic93ee267b5c2d2f2240eefdd7eb4180f3298459e
Reviewed-on: https://gerrit.instructure.com/68668
Reviewed-by: Jon Jensen <jon@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@instructure.com>
Tested-by: Jenkins
The modifier while/until construction should not be preferred.
The style guide has been updated to reflect this:
https://gollum.instructure.com/Ruby-style-guidelines#While/Until
Change-Id: Ib1b8b3cf29d52729ba9d3f9066554fc421d47111
Reviewed-on: https://gerrit.instructure.com/52997
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
Product-Review: Strand McCutchen <smccutchen@instructure.com>
QA-Review: Strand McCutchen <smccutchen@instructure.com>
The cop wants to see stabbies used for single-line
lambdas, and the lambda method for mutli-line lambdas.
This is dumb cause stabby lambdas should be used everywhere!
Change-Id: I6e9c948688b7192a34a0cff37fe1d1cadcbd047f
Reviewed-on: https://gerrit.instructure.com/52877
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Reviewed-by: Cody Cutrer <cody@instructure.com>
Product-Review: Benjamin Porter <bporter@instructure.com>
QA-Review: Benjamin Porter <bporter@instructure.com>
Change-Id: I13490195e05df00d58c190761711fe6002b7d6cd
Reviewed-on: https://gerrit.instructure.com/52310
Reviewed-by: James Williams <jamesw@instructure.com>
Tested-by: Jenkins
Product-Review: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jeremy Stanley <jeremy@instructure.com>
This commit removes the ostentatious AbcSize metric from rubocop,
and changes the following severities:
- Style/RescueModifier from convention to warning
- Lint/AmbiguousRegexpLiteral from warning to convention
- Performance/Detect from convention to warning
Change-Id: I6737b4feafaab2d4f94a44cf5f750e8c1779ca38
Reviewed-on: https://gerrit.instructure.com/52220
Reviewed-by: Cameron Matheson <cameron@instructure.com>
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Tested-by: Ethan Vizitei <evizitei@instructure.com>
closes CNVS-19816
make sure we have a spec to check rendering of the plugins
setting view from now on, and explicitly require 'dynamic_form'
in the plugins controller
also removes a linter that shouldn't be on for
describing classes (or views in this case)
TEST PLAN:
1) go configure any plugin
2) it should not explode
Change-Id: I32e58a524ddb18b922ba79e20e9e7b9c1f2a8f07
Reviewed-on: https://gerrit.instructure.com/52216
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins
QA-Review: Ethan Vizitei <evizitei@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
Change-Id: I5e1cea86576ae1787b3c837d4594d0a7c3e687d8
Reviewed-on: https://gerrit.instructure.com/51991
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
Tested-by: Jenkins
QA-Review: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Dana Danger <dana@instructure.com>
gaojun