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

Types on test settings #5643

Merged
merged 8 commits into from
Mar 6, 2025
Merged

Types on test settings #5643

merged 8 commits into from
Mar 6, 2025

Conversation

jessfraz
Copy link
Contributor

@jessfraz jessfraz commented Mar 5, 2025

part of #5625

this helps with #4919 migration as well

Copy link

vercel bot commented Mar 5, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Mar 5, 2025 10:09pm

Copy link

qa-wolf bot commented Mar 5, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

jessfraz added 2 commits March 5, 2025 12:18
Signed-off-by: Jess Frazelle <[email protected]>
Signed-off-by: Jess Frazelle <[email protected]>
@jessfraz jessfraz force-pushed the types-on-test-settings branch from 4d545ee to 53be9bf Compare March 5, 2025 20:18
Signed-off-by: Jess Frazelle <[email protected]>
@jessfraz jessfraz force-pushed the types-on-test-settings branch from 978b08e to c834898 Compare March 5, 2025 20:26
@jessfraz jessfraz marked this pull request as ready for review March 5, 2025 20:26
@jessfraz jessfraz requested review from lf94 and franknoirot March 5, 2025 20:26
@jessfraz jessfraz enabled auto-merge (squash) March 5, 2025 20:37
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.69%. Comparing base (bcac4d3) to head (009bd25).
Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5643      +/-   ##
==========================================
+ Coverage   85.67%   85.69%   +0.01%     
==========================================
  Files         106      106              
  Lines       41209    41209              
==========================================
+ Hits        35304    35312       +8     
+ Misses       5905     5897       -8     
Flag Coverage Δ
rust 85.69% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lf94
Copy link
Contributor

lf94 commented Mar 5, 2025

Looks like real test failure? Settings related

Signed-off-by: Jess Frazelle <[email protected]>
@jessfraz
Copy link
Contributor Author

jessfraz commented Mar 5, 2025

@lf94 yeah just fixed :) made it even more type proof haha

Copy link
Contributor

@lf94 lf94 left a comment

Choose a reason for hiding this comment

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

LGTM just one thing the snake_case in JS universe seems weird, like shouldn't our settingsToToml consume a JS object that uses JS naming ?

Not a blocker IMO, I'll take anything that makes settings less fucky :D.

@@ -85,7 +86,7 @@ export class AuthenticatedTronApp {
fixtures: Partial<Fixtures>
folderSetupFn?: (projectDirName: string) => Promise<void>
cleanProjectDir?: boolean
appSettings?: Partial<SaveSettingsPayload>
appSettings?: DeepPartial<Settings>
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh my god this is SO MUCH NICER

@@ -12,9 +12,10 @@ import { CmdBarFixture } from './cmdBarFixture'
import { EditorFixture } from './editorFixture'
import { ToolbarFixture } from './toolbarFixture'
import { SceneFixture } from './sceneFixture'
import { SaveSettingsPayload } from 'lib/settings/settingsTypes'
Copy link
Contributor

Choose a reason for hiding this comment

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

GOOD BYE

@@ -22,7 +26,7 @@ test.describe('Onboarding tests', () => {
{
appSettings: {
app: {
onboardingStatus: '',
onboarding_status: '',
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, isn't it weird this isnt camelCase?...

@jessfraz jessfraz merged commit 8aa4609 into main Mar 6, 2025
34 checks passed
@jessfraz jessfraz deleted the types-on-test-settings branch March 6, 2025 02:03
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