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 wrong date formats in jest tests #11996

Merged
merged 12 commits into from
Jul 17, 2024
Merged

Fix wrong date formats in jest tests #11996

merged 12 commits into from
Jul 17, 2024

Conversation

cissyxyu
Copy link
Contributor

@cissyxyu cissyxyu commented Jul 1, 2024

WHAT

Fix jest test date formatting errors.

WHY

They were causing warnings when running jest tests because npm now requires all dates to be formatted in standard UTC format.

HOW

Fix jest test date strings to be standard UTC format.

Screenshots

(If applicable. Also, please censor any sensitive data)

Notion Card Links

https://www.notion.so/quill/Console-log-cleanup-value-provided-is-not-in-a-recognized-RFC2822-or-ISO-format-9-0abd6e81e3eb420ab72ca03a04b4d591?pvs=4

What have you done to QA this feature?

Not a feature, just some test fixes. Now when I run npx jest --watch and run all tests I'm not getting the console errors.

PR Checklist Your Answer
Have you added and/or updated tests? YES
Have you deployed to Staging? No - not a feature change.
Self-Review: Have you done an initial self-review of the code below on Github? Yes

@cissyxyu cissyxyu closed this Jul 1, 2024
@cissyxyu cissyxyu reopened this Jul 1, 2024
@cissyxyu cissyxyu changed the title Jest bug Fix wrong date formats in jest tests Jul 1, 2024
@cissyxyu cissyxyu requested review from emilia-friedberg and eadams17 and removed request for emilia-friedberg July 1, 2024 15:46
Copy link
Member

@emilia-friedberg emilia-friedberg left a comment

Choose a reason for hiding this comment

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

Hey Cissy! The test fixes here look good, but I think the idea of this card (which evidently should have been clearer) is to address console.log errors that are surfacing in Jest tests. Sometimes these are false positives (Jest-only issues), but in this case it looks to me like we actually are seeing this console errors in the browser as well, so I think we need to address the errors in the actual component.

@cissyxyu
Copy link
Contributor Author

cissyxyu commented Jul 4, 2024

Hey Cissy! The test fixes here look good, but I think the idea of this card (which evidently should have been clearer) is to address console.log errors that are surfacing in Jest tests. Sometimes these are false positives (Jest-only issues), but in this case it looks to me like we actually are seeing this console errors in the browser as well, so I think we need to address the errors in the actual component.

Hi Emilia! I was able to find one actual browser instance of this error in the assigning activities flow (the publication date was being sent from the backend in the wrong format). That one bug was causing all of the console errors in the activity assignment flow. I fixed this and pushed it to this PR.

However I wasn't able to replicate the other jest bugs in the actual console after clicking around on the front end a bunch (in their respective components that were throwing the old Jest errors). Do you recall which of the components or pages seemed to be throwing the console errors?

Copy link
Member

@emilia-friedberg emilia-friedberg left a comment

Choose a reason for hiding this comment

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

Hey Cissy -- I think I just checked the first instance in the PR (assign_new_activities), which it looks like is the one you found the other fix for? If you checked everything else in the browser and they look good I think this is good to go! Thanks for your work on this.

@cissyxyu cissyxyu merged commit 00ea9bc into develop Jul 17, 2024
16 checks passed
@cissyxyu cissyxyu deleted the jest-bug branch July 17, 2024 05:27
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.

None yet

3 participants