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: clean up tests #2484

Merged
merged 4 commits into from
Dec 5, 2022
Merged

chore: clean up tests #2484

merged 4 commits into from
Dec 5, 2022

Conversation

kevinrenskers
Copy link
Contributor

@kevinrenskers kevinrenskers commented Dec 5, 2022

Two things:

  1. That delayDispatches was unused so could simply be removed
  2. Every single test in SentryFileManagerTests ended up running the deleteOldEnvelopesFromAllSentryPaths code, even though it only needed to be tested once.

#skip-changelog

@codecov
Copy link

codecov bot commented Dec 5, 2022

Codecov Report

❗ No coverage uploaded for pull request base (8.0.0@c6fb5a9). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head 03e5663 differs from pull request most recent head 3ba64df. Consider uploading reports for the commit 3ba64df to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             8.0.0   #2484   +/-   ##
=======================================
  Coverage         ?   0.00%           
=======================================
  Files            ?     178           
  Lines            ?   13189           
  Branches         ?    4922           
=======================================
  Hits             ?       0           
  Misses           ?   13185           
  Partials         ?       4           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c6fb5a9...3ba64df. Read the comment docs.

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1224.20 ms 1243.37 ms 19.16 ms
Size 20.75 KiB 383.61 KiB 362.85 KiB

Baseline results on branch: 8.0.0

Startup times

Revision Plain With Sentry Diff
a9e77dc 1231.94 ms 1254.85 ms 22.91 ms
8361c4c 1204.07 ms 1252.74 ms 48.67 ms
cdf9acd 1219.62 ms 1254.80 ms 35.18 ms
b9c9598 1228.57 ms 1251.10 ms 22.53 ms
034ff5e 1231.22 ms 1255.37 ms 24.14 ms
dd266f5 1224.76 ms 1252.63 ms 27.87 ms
2b42e4e 1212.52 ms 1249.24 ms 36.72 ms
bbf5334 1216.84 ms 1238.82 ms 21.98 ms
fb9f27a 1218.55 ms 1249.17 ms 30.62 ms
b9c9598 1232.86 ms 1253.84 ms 20.98 ms

App size

Revision Plain With Sentry Diff
a9e77dc 20.75 KiB 379.12 KiB 358.36 KiB
8361c4c 20.75 KiB 383.87 KiB 363.12 KiB
cdf9acd 20.75 KiB 383.78 KiB 363.03 KiB
b9c9598 20.75 KiB 383.77 KiB 363.01 KiB
034ff5e 20.75 KiB 383.83 KiB 363.07 KiB
dd266f5 20.75 KiB 383.39 KiB 362.64 KiB
2b42e4e 20.75 KiB 383.89 KiB 363.14 KiB
bbf5334 20.75 KiB 383.37 KiB 362.62 KiB
fb9f27a 20.75 KiB 382.11 KiB 361.36 KiB
b9c9598 20.75 KiB 383.77 KiB 363.01 KiB

Previous results on branch: chore/clean-up-test

Startup times

Revision Plain With Sentry Diff
9d49cda 1231.98 ms 1244.47 ms 12.49 ms
c5a451e 1235.24 ms 1260.56 ms 25.32 ms

App size

Revision Plain With Sentry Diff
9d49cda 20.75 KiB 383.60 KiB 362.85 KiB
c5a451e 20.75 KiB 383.61 KiB 362.85 KiB

@kevinrenskers
Copy link
Contributor Author

will decrease coverage by 88.27%.

lol

@philipphofmann
Copy link
Member

I think merging 8.0.0 into this branch could fix the codecov upload because it includes #2483.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM

@kevinrenskers kevinrenskers merged commit 57e140e into 8.0.0 Dec 5, 2022
@kevinrenskers kevinrenskers deleted the chore/clean-up-test branch December 5, 2022 12: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.

2 participants