feat: testing framework POC #959
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #959 +/- ##
==========================================
+ Coverage 83.21% 83.77% +0.56%
==========================================
Files 400 406 +6
Lines 8697 8722 +25
Branches 1798 1800 +2
==========================================
+ Hits 7237 7307 +70
+ Misses 1422 1377 -45
Partials 38 38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This reverts commit 14c2d74.
| * Sets enterpriseCuration.canOnlyViewHighlightSets to false if there are no highlight sets | ||
| * when the user enters content highlights dashboard. | ||
| */ | ||
| const setDefault = useCallback(async () => { |
There was a problem hiding this comment.
[curious] Can you expand on the rationale for this change? Why can't we define setDefault within the useEffect here?
| @@ -0,0 +1,12 @@ | |||
| import { camelCaseObject } from '@edx/frontend-platform'; | |||
|
|
|||
| const rawTestCourseData = require('./testCourseData.json'); | |||
There was a problem hiding this comment.
Might recommend looking into rosie (https://github.com/rosiejs/rosie) to generate these test data via factories instead of hardcoded JSON files.
| initialStateValue, | ||
| testCourseAggregation, | ||
| } from '../../../../data/tests/ContentHighlightsTestData'; | ||
| import 'jest-canvas-mock'; |
There was a problem hiding this comment.
[suggestion] Might recommend putting this in setupTest.js so we load the canvas mocks for Jest across all tests in the repo, not just this one test file (similar to how to use jest-localstorage-mock).
| const dropdownButton = screen.getByRole('button', { name: '1 of 5' }); | ||
| userEvent.click(dropdownButton); | ||
| const number4 = screen.getByRole('button', { name: '4' }); | ||
| // expect(number4).toBeInTheDocument(); |
There was a problem hiding this comment.
nit: remove commented out line
| import { ContentHighlightsContext, initialStateValue, testCourseAggregation } from '../../../../data/tests/ContentHighlightsTestData'; | ||
| import { useContentHighlightsContext } from '../hooks'; | ||
|
|
||
| const TestComponent = () => { |
There was a problem hiding this comment.
nit: Is there a more explicit component name that might better describe its purpose than TestComponent?
| const rawTestCourseData = require('./testCourseData.json'); | ||
| const rawTestCourseHighlights = require('./testCourseHighlights.json'); | ||
| const testCourseAggregation = require('./testCourseAggregation.json'); | ||
| const testHighlightSet = require('./testHighlightSet.json'); |
There was a problem hiding this comment.
similar to another comment, might be worth looking into rosie or similar to generate JS objects using a factory, which would likely be a bit more maintainable/scalable/customizable.
| "course:HarvardX+CS50AI": true, | ||
| "course:HarvardX+CS50P": true, | ||
| "course:HarvardX+CS50x": true | ||
| } No newline at end of file |
There was a problem hiding this comment.
nit: The intent of this file isn't immediately obvious. Is there a more explicit file name beyond "course aggregation" to indicate what this is used for?
| @@ -0,0 +1,63 @@ | |||
| /* eslint-disable max-len */ | |||
| /* eslint-disable react/jsx-filename-extension */ | |||
There was a problem hiding this comment.
We shouldn't ignore this rule. If this file is rendering JSX, it should be using the .jsx file extension.
| enterpriseId: PropTypes.string, | ||
| }), | ||
| }), | ||
| value: PropTypes.shape(), |
There was a problem hiding this comment.
[nit/curious] Should/can these empty PropTypes.shape() define their properties?
| @@ -0,0 +1,6 @@ | |||
| import { EnterpriseAppContext, initialStateValue } from './context'; | |||
|
|
|||
| export default { | |||
There was a problem hiding this comment.
nit: If you're treating the default export as an object, you might use named exports instead with something like:
export { initialStateValue, EnterpriseAppContext } from './context';
brobro10000
force-pushed
the
hu/testing
branch
from
101f0a6 to
6a5c9a3
Compare
|
No longer needed. |
Initial consolidation of testing framework POC. Consolidate contexts, constants and API structures.
A reference tech is Pact that does a similar process with HTTP responses, while this frameworks hopes to mimic it with frontend context providers. It will allow for faster testing across features that have been consolidated.
Current POC is using Content Highlights. Currently WIP.
Pushes will look at the effect on test coverage and possible testing runtimes since we are reducing the overall amount of imports required per test.
For all changes
Only if submitting a visual change