Find/fix/prevent weird stuff we are doing (or not doing) with `expect`.
Chapter I:
"I have been bent and broken, but - I hope - into a better shape."
Disallow `expect`s in a `before`. `before` is by definition before the
test, so it's not the place to be asserting anything. You can of course
still set up mocks there.
Chapter II:
"Take nothing on its looks; take everything on evidence. There's no
better rule."
Ensure every `expect` actually runs. i.e. it needs `to` or `not_to` in
order to actually do anything.
Chapter III:
"Ask no questions, and you'll be told no lies."
In selenium land, ensure every spec has at least one expecation. In
regular rspec land we just warn to stderr, but we may get more strict
in the future.
Test Plan: Specs
Change-Id: I5fc353ee8171e5191853d45f42ae42478f9220b4
Reviewed-on: https://gerrit.instructure.com/101224
Tested-by: Jenkins
Reviewed-by: Rob Orton <rob@instructure.com>
Reviewed-by: Landon Wilkins <lwilkins@instructure.com>
Product-Review: Jon Jensen <jon@instructure.com>
QA-Review: Jon Jensen <jon@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>
test plan:
* enroll a user
* use the course as that user (giving a total activity time)
* add the user to another section of the course
* the total activity time should be unchanged
* repeat, only instead of adding to another section,
switch the user to a different section (removing
them from the old one)
* total activity time should still be unchanged
closes #CNVS-16335
Change-Id: I7b7889832ae97e3d8fafe0fd1243a6048e359645
Reviewed-on: https://gerrit.instructure.com/52248
Reviewed-by: Braden Anderson <braden@instructure.com>
Tested-by: Jenkins
Reviewed-by: Jeremy Stanley <jeremy@instructure.com>
Product-Review: Jeremy Stanley <jeremy@instructure.com>
QA-Review: Jahnavi Yetukuri <jyetukuri@instructure.com>
Change-Id: I167620165171b6fd0d41e69590e1f3225ac2fe2c
Reviewed-on: https://gerrit.instructure.com/45690
Reviewed-by: Cody Cutrer <cody@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
fixes #CNVS-12934
When attempting to view a course that is
unpublished, or is published but with course start
and end override dates in the future,
it gives an unauthorized page,
but the "Last Activity" field in the course
roster registers your access. Teachers are
reading it as students are accessing courses
when they shouldn't be able to, and
contacting support.
Also refactors out "recent activity" from
the enrollment model into a class of it's own
this ALSO solves a recurring problem present
on master with the "content_migrations_spec"
which causes a spec group to fail only
when run with other specs, but not when
you run it in isolation, due to the
caching of Account.default.
TEST PLAN:
- Create a course and edit the start and end
dates on it to be in the future and check
the box "Users can only participate in the
course between these dates"
- Add a user to the course that already exists
on the institution as a student
- Masquerade as this user and go to the courses
homepage URL manually
- Stop masquerading after you receive the
un-authorized page and check the course roster
- The student you invited should still be pending,
and SHOULD NOT have a "Latest Activity" time.
Change-Id: Ifc03e79b2eb2d4933dffb11d167d20734facd490
Reviewed-on: https://gerrit.instructure.com/40855
Tested-by: Jenkins <jenkins@instructure.com>
Reviewed-by: Rob Orton <rob@instructure.com>
QA-Review: August Thornton <august@instructure.com>
Product-Review: Ethan Vizitei <evizitei@instructure.com>
fixes CAT-147
add support for a user_id param when querying course/section enrollments
ensure course and enrollment endpoints allow users to see their own
*active* enrollments in unpublished course (i.e. self-enrollments), even
if they are not admins.
slightly tighten up User#current_and_future_enrollments (only return non-
admin enrollments in unpublished courses if they are active). this method
is only used currently around self-enrollment functionality, and that sets
the enrollment state to :active, so there should be no changes in
functionality
test plan:
1. set up self-enrollment in an unpublished course
2. self-enroll as a student (via link from course settings page)
3. it should work (though you can't get into the course yet)
4. as the student, hit /api/v1/courses
1. with no params, you shouldn't see the course
2. with state[]=unpublished you should see the course
5. as the student, hit /api/v1/courses/:id/enrollments?user_id=self&state[]=current_and_future
1. you should get back your enrollment
6. as a teacher, publish the course
7. as the student, hit /api/v1/courses
1. you should get back the course
8. as the student, hit /api/v1/courses/:id/enrollments?user_id=self
1. you should get back your enrollment
Change-Id: I75a4f77b0298729088c7f8e3dbc46d551f36f241
Reviewed-on: https://gerrit.instructure.com/35085
Reviewed-by: Jeff Belser <jbelser@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Marc LeGendre <marc@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
they're not relevant to *a* User, and they're needed by things other than
User.reflections.
test plan:
n/a, just refactoring to a (hopefully) better spot. existing and new specs
should cover all of this
Change-Id: I66ab5314582113ac549af5662bcf08a8ca5df580
Reviewed-on: https://gerrit.instructure.com/35082
Reviewed-by: Dave Donahue <ddonahue@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>
Product-Review: Marc LeGendre <marc@instructure.com>
QA-Review: Marc LeGendre <marc@instructure.com>
Cody Cutrer