-
Notifications
You must be signed in to change notification settings - Fork 480
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
Move integration tests #45175
Move integration tests #45175
Conversation
// Also calls firehose event on submit, but we reload the page on submit | ||
// The page reload can't be stubbed because `window.location.reload` | ||
// is not writable nor configurable: Object.getOwnPropertyDescriptor(window.location, 'toString') | ||
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty#modifying_a_property |
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 spent too long on this 😅 . We could do the window.location.reload in a separate function that we stub out, but then we'd have to export that function for the test here. I don't think there's a "good" way to test this––open to ideas
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.
so we actually do have a util already specifically for this purpose:
code-dot-org/apps/src/utils.js
Lines 725 to 727 in 1274ce4
export function reload() { | |
window.location.reload(); | |
} |
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.
Ooo excellent––will update this!
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 think this is fine until we get a more holistic integration test approach. I don't think we'd want to run these separately (different karma/mocha config) anyways
thank you for doing this Meg! This seems like a step in the right direction. it would be great for our engineering team to get on the same page about js integration tests. |
As part of the TeacherApplication test refactor, I moved the tests requiring
mount
to a separate file. I originally had these in theintegration
directory, but I had to import enzyme for it to work (see 14d5c23). Instead, I kept it in theunit
directory but with a different name.Links
Testing story
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: