Skip to content

chore(deps): update paragon and frontend-build to openedx scope#818

Merged
arbrandes merged 1 commit intoopenedx:masterfrom
brian-smith-tcril:use-openedx-scope
Feb 16, 2024
Merged

chore(deps): update paragon and frontend-build to openedx scope#818
arbrandes merged 1 commit intoopenedx:masterfrom
brian-smith-tcril:use-openedx-scope

Conversation

@brian-smith-tcril
Copy link
Contributor

No description provided.

"@edx/react-unit-test-utils": "^2.0.0",
"@edx/reactifex": "^1.0.3",
"@edx/stylelint-config-edx": "^2.3.0",
"@edx/stylelint-config-edx": "2.3.0",
Copy link
Contributor Author

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.

Copy link
Contributor

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.

Comment on lines +52 to +65
class ResizeObserver {
observe() {
// do nothing
}

unobserve() {
// do nothing
}

disconnect() {
// do nothing
}
}

window.ResizeObserver = ResizeObserver;
Copy link
Contributor Author

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.

const { container } = renderComponent();
expect(container).toBeEmptyDOMElement();
const { queryByTestId } = renderComponent();
expect(queryByTestId('browser-router')).toBeEmptyDOMElement();
Copy link
Contributor Author

@brian-smith-tcril brian-smith-tcril Feb 16, 2024

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

@brian-smith-tcril brian-smith-tcril marked this pull request as ready for review February 16, 2024 00:16
@codecov
Copy link

codecov bot commented Feb 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (51c5f9c) 89.27% compared to head (aeb0ee5) 89.27%.

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.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@arbrandes arbrandes left a 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!

@arbrandes arbrandes merged commit 76bb8e8 into openedx:master Feb 16, 2024
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

Comments