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

chore(ui): Change testEnvironment from node to jsdom #16287

Merged
merged 2 commits into from
Nov 9, 2023

Conversation

rpelczar
Copy link
Contributor

@rpelczar rpelczar commented Nov 9, 2023

Tests in some PRs failed because the testEnvironment option was set incorrectly. I have no idea why the tests sometimes passed. This change should fix that.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
  • The title of the PR states what changed and the related issues number (used for the release note).
  • The title of the PR conforms to the Toolchain Guide
  • I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
  • I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
  • Does this PR require documentation updates?
  • I've updated documentation as required by this PR.
  • Optional. My organization is added to USERS.md.
  • I have signed off all my commits as required by DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My build is green (troubleshooting builds).
  • My new feature complies with the feature status guidelines.
  • I have added a brief description of why this PR is necessary and/or what this PR solves.

@rpelczar rpelczar requested a review from a team as a code owner November 9, 2023 01:38
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (58da6a3) 49.51% compared to head (a7284f2) 49.52%.
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #16287   +/-   ##
=======================================
  Coverage   49.51%   49.52%           
=======================================
  Files         269      269           
  Lines       46990    46990           
=======================================
+ Hits        23269    23272    +3     
+ Misses      21437    21435    -2     
+ Partials     2284     2283    -1     

see 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@matthewhughes-uw
Copy link
Contributor

Thanks for this! I was wondering why these tests were failing in CI for my non-UI change.

I have no idea why the tests sometimes passed. This change should fix that.

This was also confusing me. For what it's worth I can reproduce this failure everytime locally if I run yarn test --no-cache however with caching it will fail the first time and then succeed every time after that. So if CI was storing jest's cache I guess this could explain it sometimes failing and sometimes not.

Copy link
Collaborator

@alexmt alexmt left a comment

Choose a reason for hiding this comment

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

Thank you!

@alexmt alexmt merged commit b33b7fa into argoproj:master Nov 9, 2023
24 checks passed
jmilic1 pushed a commit to jmilic1/argo-cd that referenced this pull request Nov 13, 2023
alexmt pushed a commit to alexmt/argo-cd that referenced this pull request Nov 27, 2023
alexmt added a commit that referenced this pull request Nov 27, 2023
* chore(cli): clarify core mode code (#15800)

* chore(cli): clarify core mode code

Signed-off-by: Michael Crenshaw <[email protected]>

* rename function

Signed-off-by: Michael Crenshaw <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>

* fix: fixed cli admin dashboard cmd (#16430)

* fix: fixed cli admin dashboard cmd

Signed-off-by: Soumya Ghosh Dastidar <[email protected]>

* feat: update docs

Signed-off-by: Soumya Ghosh Dastidar <[email protected]>

---------

Signed-off-by: Soumya Ghosh Dastidar <[email protected]>

* chore(ui): Change testEnvironment from node to jsdom (#16287)

Signed-off-by: Rafal Pelczar <[email protected]>

---------

Signed-off-by: Michael Crenshaw <[email protected]>
Signed-off-by: Soumya Ghosh Dastidar <[email protected]>
Signed-off-by: Rafal Pelczar <[email protected]>
Co-authored-by: Michael Crenshaw <[email protected]>
Co-authored-by: Soumya Ghosh Dastidar <[email protected]>
Co-authored-by: Rafal <[email protected]>
vladfr pushed a commit to vladfr/argo-cd that referenced this pull request Dec 13, 2023
tesla59 pushed a commit to tesla59/argo-cd that referenced this pull request Dec 16, 2023
lyda pushed a commit to lyda/argo-cd that referenced this pull request Mar 28, 2024
Hariharasuthan99 pushed a commit to AmadeusITGroup/argo-cd that referenced this pull request Jun 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.

3 participants