Persist theme - Reload everything on a disconnect #3250

lf94 · 2024-08-03T18:30:05Z

Closes #3106 (@franknoirot didn't set you as a reviewer on this one since I know you're traveling atm)

qa-wolf · 2024-08-03T18:30:08Z

A coverage request was created for this pull request.

vercel · 2024-08-03T18:30:08Z

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

Name	Status	Preview	Updated (UTC)
modeling-app	✅ Ready (Inspect)	Visit Preview	Aug 7, 2024 7:03am

Irev-Dev · 2024-08-03T22:52:06Z

Is it possible that this broke the ability of the unit tests to connect to the engine? because build-test-web https://github.com/KittyCAD/modeling-app/actions/runs/10230111319/job/28304548336?pr=3250 failed and I tried a re-run.

Other branches are passing just fine so think something has gone awry here
#3254

I will try and take a look but you can run this test locally with yarn test:nowatch artifactGraph

jessfraz · 2024-08-04T02:27:34Z

We could do a playwright snapshot test here or otherwise make sure qawolf adds coverage

Irev-Dev · 2024-08-04T21:25:36Z

Lee FYI I'll take a look at fixing the tests this morning.

Irev-Dev · 2024-08-04T23:27:29Z

src/lang/std/engineConnection.ts

+          if (!this.pingPongSpan.ping)
+            this.connect(this.engineCommandManager.disableWebRTC ? true : false)


@lf94 This is what I did to fix the unit tests (build-test-web), because I wasn't able to replicate the original issue i.e. #3265 would you mind double checking this doesn't cause other issues?

Only seeing this comment now. Let's see what happens with the latest changes

@lf94 replying to
#3250 (comment)

This is my fix for the failing artifact map unit tests

Oh but it's broken again hmmm.

Irev-Dev · 2024-08-04T23:28:17Z

@jessfraz I have a test for this ready to go, just hit another issue is all
#3264

Irev-Dev

Actually I think #3265 is not a separate issue, but is introduced by this change as Test network and connection issues › simulate network down and network little widget seems to be consistently failing

…nnects

lf94 · 2024-08-06T04:00:35Z

@Irev-Dev what's the artifact map stuff failing in the web test?

Irev-Dev · 2024-08-06T04:39:17Z

@Irev-Dev what's the artifact map stuff failing in the web test?

So failure here
https://github.com/KittyCAD/modeling-app/actions/runs/10258606032/job/28381694429?pr=3250

Basically that test tries to connect to the real-dev engine websockets, but without webRTC, I did manage to fix it once earlier on this branch, but it's seems to be broken again not sure why.

lf94 · 2024-08-06T13:33:47Z

Ok should be no problem to fix

lf94 · 2024-08-06T14:09:31Z

Ah the issue must be because I removed the await and instead added this in the artifactGraph.test

  engineCommandManager.addEventListener(
    EngineCommandManagerEvents.SceneReady,
    async () => {

* kurts attempts * we're almost sane * get tests working, praise be --------- Co-authored-by: 49lf <ircsurfer33@gmail.com>

Irev-Dev · 2024-08-07T06:59:02Z

So much effort to make the theme persist haha.

This reverts commit 3f082c8.

lf94 requested review from jtran, jessfraz and Irev-Dev August 3, 2024 18:30

vercel bot deployed to Preview August 3, 2024 18:50 View deployment

vercel bot deployed to Preview August 4, 2024 22:09 View deployment

Irev-Dev changed the title ~~Reload everything on a disconnect~~ Persist theme - Reload everything on a disconnect Aug 4, 2024

This was referenced Aug 4, 2024

Test for reconnect and theme #3264

Merged

Reconnection issue (theme change) #3265

Closed

Irev-Dev reviewed Aug 4, 2024

View reviewed changes

vercel bot deployed to Preview August 4, 2024 23:28 View deployment

Irev-Dev requested changes Aug 5, 2024

View reviewed changes

lf94 and others added 2 commits August 5, 2024 10:30

Reload everything on a disconnect

09d14d8

fix unit-integration tests

0561784

lf94 force-pushed the theme-not-persist branch from a4572d7 to 27db603 Compare August 6, 2024 01:10

Further improvements to connection manager; persist theme across reco…

27db603

…nnects

vercel bot deployed to Preview August 6, 2024 01:20 View deployment

lf94 requested a review from Irev-Dev August 6, 2024 01:20

vercel bot deployed to Preview August 6, 2024 01:31 View deployment

vercel bot deployed to Preview August 6, 2024 12:34 View deployment

lf94 force-pushed the theme-not-persist branch from 14b2503 to 0e38477 Compare August 6, 2024 21:44

lf94 added 2 commits August 6, 2024 17:45

Fix up artifactGraph.test

0e38477

Actually pass the callback

427d923

vercel bot deployed to Preview August 6, 2024 22:06 View deployment

Kurt hmmm (#3308)

eb6df02

* kurts attempts * we're almost sane * get tests working, praise be --------- Co-authored-by: 49lf <ircsurfer33@gmail.com>

Irev-Dev approved these changes Aug 7, 2024

View reviewed changes

Merge branch 'main' into theme-not-persist

311c247

vercel bot deployed to Preview August 7, 2024 06:26 View deployment

typo

53499db

vercel bot deployed to Preview August 7, 2024 07:03 View deployment

Irev-Dev merged commit 3f082c8 into main Aug 7, 2024
21 checks passed

Irev-Dev deleted the theme-not-persist branch August 7, 2024 07:11

This was referenced Aug 7, 2024

Cut release v0.24.10 #3309

Merged

Switch projects fix #3310

Merged

franknoirot mentioned this pull request Aug 7, 2024

Don't set firstRender from within a routeLoader #3311

Merged

franknoirot added a commit that referenced this pull request Aug 7, 2024

Revert "Persist theme - Reload everything on a disconnect (#3250)"

1e80738

This reverts commit 3f082c8.

franknoirot mentioned this pull request Aug 7, 2024

Cut release v0.24.10 #3314

Closed

Irev-Dev mentioned this pull request Aug 8, 2024

Cut release v0.24.10 #3323

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Persist theme - Reload everything on a disconnect #3250

Persist theme - Reload everything on a disconnect #3250

lf94 commented Aug 3, 2024 •

edited

Loading

qa-wolf bot commented Aug 3, 2024 •

edited

Loading

vercel bot commented Aug 3, 2024 •

edited

Loading

Irev-Dev commented Aug 3, 2024 •

edited

Loading

jessfraz commented Aug 4, 2024

Irev-Dev commented Aug 4, 2024

Irev-Dev Aug 4, 2024 •

edited

Loading

lf94 Aug 6, 2024

Irev-Dev Aug 6, 2024

Irev-Dev Aug 6, 2024

Irev-Dev commented Aug 4, 2024

Irev-Dev left a comment

lf94 commented Aug 6, 2024

Irev-Dev commented Aug 6, 2024

lf94 commented Aug 6, 2024

lf94 commented Aug 6, 2024

Irev-Dev commented Aug 7, 2024

		if (!this.pingPongSpan.ping)
		this.connect(this.engineCommandManager.disableWebRTC ? true : false)

Persist theme - Reload everything on a disconnect #3250

Persist theme - Reload everything on a disconnect #3250

Conversation

lf94 commented Aug 3, 2024 • edited Loading

qa-wolf bot commented Aug 3, 2024 • edited Loading

vercel bot commented Aug 3, 2024 • edited Loading

Irev-Dev commented Aug 3, 2024 • edited Loading

jessfraz commented Aug 4, 2024

Irev-Dev commented Aug 4, 2024

Irev-Dev Aug 4, 2024 • edited Loading

Choose a reason for hiding this comment

lf94 Aug 6, 2024

Choose a reason for hiding this comment

Irev-Dev Aug 6, 2024

Choose a reason for hiding this comment

Irev-Dev Aug 6, 2024

Choose a reason for hiding this comment

Irev-Dev commented Aug 4, 2024

Irev-Dev left a comment

Choose a reason for hiding this comment

lf94 commented Aug 6, 2024

Irev-Dev commented Aug 6, 2024

lf94 commented Aug 6, 2024

lf94 commented Aug 6, 2024

Irev-Dev commented Aug 7, 2024

lf94 commented Aug 3, 2024 •

edited

Loading

qa-wolf bot commented Aug 3, 2024 •

edited

Loading

vercel bot commented Aug 3, 2024 •

edited

Loading

Irev-Dev commented Aug 3, 2024 •

edited

Loading

Irev-Dev Aug 4, 2024 •

edited

Loading