-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(deps): update paragon and frontend-build to openedx scope #818
Conversation
ff22705
to
304445e
Compare
"@edx/reactifex": "^1.0.3", | ||
"@edx/stylelint-config-edx": "^2.3.0", | ||
"@edx/stylelint-config-edx": "2.3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was on 2.3.0
due to the package-lock.json
, after rebuilding the package lock to cleanly handle the scope changes it updated to 2.3.3
and caused https://github.com/openedx/frontend-app-course-authoring/actions/runs/7746261151/job/21124075247 to fail with errors such as
src/index.scss
1:9 ✖ Expected ""~@edx/brand/paragon/fonts"" to be "url("~@edx/brand/paragon/fonts")" import-notation
Pinning it to 2.3.0
prevents those errors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No objections. We can deal with that later.
08ceb04
to
8ae9a9c
Compare
42b105d
to
16720b4
Compare
16720b4
to
ae163a8
Compare
class ResizeObserver { | ||
observe() { | ||
// do nothing | ||
} | ||
|
||
unobserve() { | ||
// do nothing | ||
} | ||
|
||
disconnect() { | ||
// do nothing | ||
} | ||
} | ||
|
||
window.ResizeObserver = ResizeObserver; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was getting "ResizeObserver is not defined" errors. I decided to see how paragon is handling those and I found https://github.com/openedx/paragon/blob/7e4a81f9f2350c54362baa852c75dc32c2c2694e/src/setupTest.js#L7-L21, so I copied that here.
c140298
to
5b4e48b
Compare
5b4e48b
to
4783c82
Compare
29e5244
to
aeb0ee5
Compare
const { container } = renderComponent(); | ||
expect(container).toBeEmptyDOMElement(); | ||
const { queryByTestId } = renderComponent(); | ||
expect(queryByTestId('browser-router')).toBeEmptyDOMElement(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of openedx/frontend-platform#615, the AppProvider
used in renderComponent
includes
<div data-testid="browser-router">
and the OptionalReduxProvider
in the AppProvider
includes
<div data-testid="redux-provider">
so instead of the whole container being an empty DOM element, it is
<div data-testid="redux-provider"><div data-testid="browser-router"></div></div>
This just updates the test to ensure nothing is rendered inside those divs that are always rendered
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #818 +/- ##
=======================================
Coverage 89.27% 89.27%
=======================================
Files 551 551
Lines 9738 9738
Branches 2099 2099
=======================================
Hits 8694 8694
Misses 996 996
Partials 48 48 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Congrats on making all tests pasts, and thanks for getting to the bottom of it!
No description provided.