Skip to content
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

Fix unit test failures on gh-actions #315

Merged
merged 10 commits into from
Sep 19, 2023
Merged

Fix unit test failures on gh-actions #315

merged 10 commits into from
Sep 19, 2023

Conversation

vincerubinetti
Copy link
Contributor

@vincerubinetti vincerubinetti commented Sep 12, 2023

  • fixes regression in apiCall util call due to unknown cause. add arbitrary timeout (see issue linked in code comment) in lieu of adding whole new package @testing-library/vue just for one function waitFor and rewriting all unit tests that use apiCall.
  • update all packages to latest versions

@netlify
Copy link

netlify bot commented Sep 12, 2023

Deploy Preview for monarch-app ready!

Name Link
🔨 Latest commit a848673
🔍 Latest deploy log https://app.netlify.com/sites/monarch-app/deploys/65048dcb861a1800084e5856
😎 Deploy Preview https://deploy-preview-315--monarch-app.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@vincerubinetti vincerubinetti changed the title Test Fix unit test failures on gh-actions Sep 12, 2023
Copy link
Collaborator

@falquaddoomi falquaddoomi left a comment

Choose a reason for hiding this comment

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

Looks good; I left a note about clarifying a bit of code, but it all looks reasonable. Seems the tests are passing now, too, so kudos. Nice catch on the configuration key for "prettier-plugin-css-order", too.

@@ -38,6 +38,7 @@ export const apiCall = async (): Promise<void> => {
*/
await sleep();
await sleep();
await sleep(10);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps a comment here about why it's 10 seconds, in addition to the two non-parameterized calls above, would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately, the 10ms is arbitrary and I really don't know why the error is happening. I can't replicate it locally.

I'll add a generic comment to this effect, also leave this PR open a little longer (maybe til next week) to keep investigating.

@vincerubinetti
Copy link
Contributor Author

Merging this so PRs stop failing, but I'm setting a reminder for myself to keep investigating.

@vincerubinetti vincerubinetti merged commit 76c20a0 into monarch-initiative:main Sep 19, 2023
8 of 9 checks passed
@vincerubinetti vincerubinetti deleted the test branch September 19, 2023 18:17
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