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

WIP: Fix bad network until a project is opened #2661

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [macos-14, ubuntu-latest, windows-latest]
os: [macos-14, windows-latest]
# TODO: add back ubuntu when https://github.com/KittyCAD/modeling-app/issues/625 is closed
# os: [macos-14, ubuntu-latest, windows-latest]
env:
# Specific Apple Universal target for macos
TAURI_ARGS_MACOS: ${{ matrix.os == 'macos-14' && '--target universal-apple-darwin' || '' }}
Expand Down
17 changes: 15 additions & 2 deletions e2e/tauri/specs/app.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import os from 'os'
import { click, setDatasetValue } from '../utils'

const isWin32 = os.platform() === 'win32'
const tauriProtocol = isWin32 ? 'http://tauri.localhost' : 'tauri://localhost'
const documentsDir = path.join(os.homedir(), 'Documents')
const userSettingsDir = path.join(
os.homedir(),
Expand Down Expand Up @@ -67,12 +68,24 @@ describe('ZMA sign in flow', () => {
await new Promise((resolve) => setTimeout(resolve, 10000))
const newFileButton = await $('[data-testid="home-new-file"]')
expect(await newFileButton.getText()).toEqual('New project')

// Refresh once before the authorized user flows
await browser.execute(`window.location.href = "${tauriProtocol}/home"`)
await new Promise((resolve) => setTimeout(resolve, 10000))
})
})

describe('ZMA authorized user flows', () => {
// Note: each flow below is intended to start *and* end from the home page

it('checks the network indicator status', async () => {
const toggle = await $('[data-testid="network-toggle"]')
await click(toggle)
const status = await $('[data-testid="network"]')
expect(await status.getText()).toEqual('CONNECTED')
await click(toggle)
})

it('opens the settings page, checks filesystem settings, and closes the settings page', async () => {
const menuButton = await $('[data-testid="user-sidebar-toggle"]')
await click(menuButton)
Expand Down Expand Up @@ -137,8 +150,8 @@ describe('ZMA authorized user flows', () => {
const errorText = await $('[data-testid="unexpected-error"]')
expect(await errorText.getText()).toContain('unexpected error')
}
const base = isWin32 ? 'http://tauri.localhost' : 'tauri://localhost'
await browser.execute(`window.location.href = "${base}/home"`)
await browser.execute(`window.location.href = "${tauriProtocol}/home"`)

})
})

Expand Down
9 changes: 6 additions & 3 deletions src/Router.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,12 @@ const router = createBrowserRouter([
path: paths.HOME,
element: (
<Auth>
<Outlet />
<Home />
<CommandBar />
<ModelingMachineProvider>
<Outlet />
<Home />
<CommandBar />
</ModelingMachineProvider>
<WasmErrBanner />
Comment on lines +137 to +142
Copy link
Collaborator Author

@pierremtb pierremtb Jun 16, 2024

Choose a reason for hiding this comment

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

Noticed that this does the trick only when you open the app signed in. Works after a refresh after signing in, which is probably why the test fails

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah forcing a refresh after auth makes the test happy.

@lf94 @franknoirot The way I tried to enable the engine connection on the home page here was a complete shot in the dark. Let me know how you'd want to proceed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, sorry for the dirty diff here. We should merge #2659 first

</Auth>
),
id: paths.HOME,
Expand Down
Loading