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

feat: Refactor and add tests for load-mock #53302

Merged
merged 3 commits into from
Jul 21, 2023
Merged

feat: Refactor and add tests for load-mock #53302

merged 3 commits into from
Jul 21, 2023

Conversation

markstory
Copy link
Member

Refactor load-mocks into a set of smaller functions, and have some high level tests to ensure that load-mocks will run. These tests aren't exhaustive but they should help us catch and prevent regressions in load-mocks in the future.

add more functions and move logic into a module that can have tests.
Refactor load-mocks into a set of smaller functions, and have some high
level tests to ensure that load-mocks will run. These tests aren't
exhaustive but they should help us catch and prevent regressions in
load-mocks in the future.
@markstory markstory requested a review from a team July 20, 2023 19:56
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 20, 2023
Copy link
Contributor

@corps corps left a comment

Choose a reason for hiding this comment

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

The diff is hard to follow, but that's understandable, it has no tests and is a dev tool. So I'm taking your word that this works locally, I do think the approach is good 👍

@codecov
Copy link

codecov bot commented Jul 20, 2023

Codecov Report

Merging #53302 (f3ccc63) into master (0b666f0) will increase coverage by 0.02%.
The diff coverage is 66.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #53302      +/-   ##
==========================================
+ Coverage   79.51%   79.54%   +0.02%     
==========================================
  Files        4939     4941       +2     
  Lines      208029   208431     +402     
  Branches    35474    35521      +47     
==========================================
+ Hits       165422   165787     +365     
- Misses      37569    37595      +26     
- Partials     5038     5049      +11     
Impacted Files Coverage Δ
static/app/components/smartSearchBar/utils.tsx 84.56% <ø> (ø)
static/app/utils/analytics/integrations/index.ts 100.00% <ø> (ø)
...tatic/app/views/releases/detail/overview/index.tsx 1.90% <ø> (ø)
.../detail/overview/sidebar/projectReleaseDetails.tsx 33.33% <ø> (ø)
...zationGeneralSettings/organizationSettingsForm.tsx 90.90% <ø> (ø)
...onIntegrations/abstractIntegrationDetailedView.tsx 75.22% <ø> (ø)
...ganizationIntegrations/integrationDetailedView.tsx 66.27% <ø> (ø)
src/sentry/utils/mockdata/core.py 66.47% <66.47%> (ø)
src/sentry/middleware/auth.py 92.20% <100.00%> (-0.10%) ⬇️
src/sentry/utils/mockdata/__init__.py 100.00% <100.00%> (ø)

... and 31 files with indirect coverage changes

I think having a few type:ignore is better than ignore the entire file.
@markstory markstory merged commit 9fd943a into master Jul 21, 2023
@markstory markstory deleted the fix-load-mocks branch July 21, 2023 20:59
@markstory
Copy link
Member Author

Ran this locally before merging and it runs cleanly.

armenzg pushed a commit that referenced this pull request Jul 24, 2023
Refactor load-mocks into a set of smaller functions, and have some high
level tests to ensure that load-mocks will run. These tests aren't
exhaustive but they should help us catch and prevent regressions in
load-mocks in the future.
@github-actions github-actions bot locked and limited conversation to collaborators Aug 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants