Skip to content

CoursewareContainer tests. - #108

Merged
davidjoy merged 8 commits into
masterfrom
djoy/courseware-container-tests
Jul 15, 2020
Merged

CoursewareContainer tests.#108
davidjoy merged 8 commits into
masterfrom
djoy/courseware-container-tests

Conversation

@davidjoy

Copy link
Copy Markdown
Contributor

This PR is best reviewed commit-by-commit. It'll be much easier to understand what's going on.

David Joy added 7 commits July 14, 2020 16:01
We need to use it in a few places.  Seems worth not-repeating, since they can easily get out of sync.  In general, tests should only test the parts of the store they care about, as well.
Ultimately I’d like to move this to frontend-platform as an alternative to ‘initialize’ for tests.  ‘initialize’ is an async function which complicates matters.
This removes the need for the snapshot file, and ensures our test is more resilient to unrelated changes in the store.

Also added a few more stages of assertions to some of the tests, showing that they have the right values over time.
We can use this in the courseware data tests, and shortly in the tests for CoursewareContainer.
This will help us test sequence navigation’s behavior more fully by having multiple units in a sequence.
@davidjoy
davidjoy requested a review from a team July 14, 2020 20:05
@davidjoy
davidjoy force-pushed the djoy/courseware-container-tests branch from 4f6eecb to 8e36477 Compare July 14, 2020 20:25
@codecov

codecov Bot commented Jul 14, 2020

Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (master@bc30b20). Click here to learn what that means.
The diff coverage is 90.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #108   +/-   ##
=========================================
  Coverage          ?   54.22%           
=========================================
  Files             ?      108           
  Lines             ?     1219           
  Branches          ?      238           
=========================================
  Hits              ?      661           
  Misses            ?      541           
  Partials          ?       17           
Impacted Files Coverage Δ
src/course-home/progress-tab/ProgressTab.jsx 0.00% <ø> (ø)
src/courseware/CoursewareContainer.jsx 73.95% <ø> (ø)
src/index.jsx 0.00% <ø> (ø)
...are/data/__factories__/sequenceMetadata.factory.js 81.81% <75.00%> (ø)
...rseware/data/__factories__/courseBlocks.factory.js 100.00% <100.00%> (ø)
src/store.js 100.00% <100.00%> (ø)
src/generic/model-store/slice.js 68.96% <0.00%> (ø)
src/CoursewareRedirect.jsx 0.00% <0.00%> (ø)
src/courseware/course/sequence/Unit.jsx 0.00% <0.00%> (ø)
.../courseware/course/bookmark/BookmarkFilledIcon.jsx 0.00% <0.00%> (ø)
... and 104 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc30b20...310fd0a. Read the comment docs.

@kdmccormick kdmccormick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Thanks for breaking it into commits. Only one piece of feedback.

expect(container.querySelector('.fake-unit')).toHaveTextContent(unitId);
});

describe('course load denied redirects', () => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you improve the describe/it strings for the course load denied redirects tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! Thanks. Should have done better on those. ;) Fixed.

Tests loading, sequence navigation/unit rendering, and ‘denied’ states.

Subsequent tests will add tests for handlers.
@davidjoy
davidjoy force-pushed the djoy/courseware-container-tests branch from 8e36477 to 310fd0a Compare July 14, 2020 21:41
@davidjoy
davidjoy merged commit afb4b77 into master Jul 15, 2020
@davidjoy
davidjoy deleted the djoy/courseware-container-tests branch July 15, 2020 14:27
hopewise pushed a commit to zeit-labs/frontend-app-learning that referenced this pull request Sep 27, 2023
…fic-assignment

add filter for specific assignment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants