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(dashboard): parse analysis values with JSON5 to handle NaN. Fixes #2758 #3801

Merged
merged 1 commit into from
Aug 22, 2024

Conversation

AppliNH
Copy link
Contributor

@AppliNH AppliNH commented Aug 22, 2024

Fixes #2758 using json5 which is already a dependency here.

Here's a Repl playground that demonstrates it's fixing the issue.

Checklist:

  • Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this is a chore.
  • The title of the PR is (a) conventional with a list of types and scopes found here, (b) states what changed, and (c) suffixes the related issues number. E.g. "fix(controller): Updates such and such. Fixes #1234".
  • I've signed my commits with DCO
  • I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
  • My builds are green. Try syncing with master if they are not.
  • My organization is added to USERS.md.

@AppliNH AppliNH changed the title fix(ui analysis-modal): parse analysis values with JSON5 to handle NaN. Fixes #2758 fix(ui): parse analysis values with JSON5 to handle NaN. Fixes #2758 Aug 22, 2024
@AppliNH AppliNH changed the title fix(ui): parse analysis values with JSON5 to handle NaN. Fixes #2758 fix(dashboard): parse analysis values with JSON5 to handle NaN. Fixes #2758 Aug 22, 2024
Signed-off-by: AppliNH <applinh@protonmail.com>
@AppliNH AppliNH force-pushed the fix-dashboard-crash-nan-value branch from 2e229f0 to ac380c5 Compare August 22, 2024 13:14
Copy link

Copy link
Contributor

Published E2E Test Results

  4 files    4 suites   3h 30m 59s ⏱️
112 tests  98 ✅  7 💤 7 ❌
456 runs  421 ✅ 28 💤 7 ❌

For more details on these failures, see this check.

Results for commit ac380c5.

Copy link
Contributor

Published Unit Test Results

2 263 tests   2 263 ✅  2m 59s ⏱️
  128 suites      0 💤
    1 files        0 ❌

Results for commit ac380c5.

Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.10%. Comparing base (c1ffa8e) to head (ac380c5).
Report is 61 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3801      +/-   ##
==========================================
+ Coverage   84.00%   84.10%   +0.09%     
==========================================
  Files         162      162              
  Lines       18517    18517              
==========================================
+ Hits        15556    15574      +18     
+ Misses       2100     2091       -9     
+ Partials      861      852       -9     

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


🚨 Try these New Features:

@zachaller zachaller merged commit 80fff8e into argoproj:master Aug 22, 2024
28 checks passed
meeech pushed a commit to CircleCI-Public/argo-rollouts that referenced this pull request Feb 10, 2025
…rgoproj#2758 (argoproj#3801)

fix(ui analysis-modal): parse analysis values with JSON5 to handle NaN

Signed-off-by: AppliNH <applinh@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dashboard crashes when result from AnalysisRun is [NaN]
2 participants