test(ws): Add workspaces tests #188

yehudit1987 · 2025-01-30T10:29:53Z

This PR resolve issue #142

ederign · 2025-02-05T19:28:59Z

@paulovmr would mind to take a look on this one?

ederign · 2025-02-05T19:29:13Z

/assign @paulovmr

thesuperzapper · 2025-02-20T16:50:09Z

/ok-to-test

thesuperzapper · 2025-03-07T02:20:58Z

@yehudit1987 this can be rebased now that #213 is merged.

The backend changes from this PR should no longer be needed, but the frontend will need to understand the new "services" part of the API from that PR.

paulovmr

Hi @yehudit1987 , thanks for the PR! I've made a partial review, and will test it as well after the backend PR is merged and this one is rebased. I've left a few comments inline. Also:

A .gitignore entry was added and already merged in the notebooks-v2 branch to avoid the workspaces/frontend/src/__tests__/cypress/cypress/downloads/downloads.htm file to be versioned. Your PR is trying to add the file. My guess is that because when you opened the PR the .gitignore entry did not exist, the download.htm file was already added to your commit, so you might need to remove it manually in your branch.

paulovmr · 2025-03-12T15:16:31Z

workspaces/frontend/src/__tests__/cypress/cypress/tests/mocked/workspace.mock.ts

+  podConfigDisplayName: string,
+  pvcName: string,
+): {
+  name: string;


Could this return type be replaced by the Workspace type directly?

paulovmr · 2025-03-12T15:24:59Z

workspaces/frontend/src/app/pages/Workspaces/Details/WorkspaceDetailsActivity.tsx

  workspace,
 }) => {
-  const { activity, pauseTime, pendingRestart } = workspace.status;
+  const { activity, pausedTime: pauseTime, pendingRestart } = workspace;


Just to keep the consistency, please consider keeping all usages as pausedTime (also label and usage in lines 38 to 40).

Suggested change

const { activity, pausedTime: pauseTime, pendingRestart } = workspace;

const { activity, pausedTime, pendingRestart } = workspace;

paulovmr · 2025-03-12T15:52:32Z

workspaces/frontend/src/app/pages/Workspaces/Details/WorkspaceDetailsOverview.tsx

      <DescriptionListTerm>Labels</DescriptionListTerm>
      <DescriptionListDescription>
-        {workspace.podTemplate.podMetadata.labels.join(', ')}
+        {workspace.podTemplate.options.podConfig.current.labels.length > 0


I believe we could leverage PatternFly Label component to better list the labels here. Since it is out of the scope of this PR, I've created a different issue for it: #232

paulovmr · 2025-03-12T15:56:22Z

workspaces/frontend/src/app/pages/Workspaces/Details/WorkspaceDetailsOverview.tsx

      <DescriptionListTerm>Pod config</DescriptionListTerm>
-      <DescriptionListDescription>{workspace.options.podConfig}</DescriptionListDescription>
+      <DescriptionListDescription>
+        {JSON.stringify(workspace.podTemplate.options.podConfig, null, 2)}


Suggested change

{JSON.stringify(workspace.podTemplate.options.podConfig, null, 2)}

{workspace.podTemplate.options.podConfig.current.displayName}

paulovmr

/lgtm

ederign

@paulovmr @yehudit1987 my suggestion is to hold this until we:

Have a better controller installation/setup with #220
Figure out where we should mock. I suggest we discuss mocking this on BFF layer to avoid dependency on controller for front-end development. After we finish a feature we can always test it integrated (ideally with automated tests and at least with tilt)

ederign · 2025-03-19T18:36:48Z

/hold

caponetto

@yehudit1987 I left one comment on the code, and I think we are good to go.

In addition:

Could you please use the test prefix for this PR instead of feat?
Could you please squash the 12 commits into a single one?

Thank you!

caponetto · 2025-05-20T17:58:51Z

workspaces/frontend/package-lock.json

      "resolved": "https://registry.npmjs.org/cross-env/-/cross-env-7.0.3.tgz",
      "integrity": "sha512-+/HKd6EgcQCJGh2PSjZuUitQBQynKor4wrFbRg4DtAgS1aWO+gU52xpH7M9ScGgXSYmAVS9bIJ8EzuaGw0oNAw==",
      "dev": true,
+      "license": "MIT",


Was this change required? It seems unnecessary for this PR.

caponetto · 2025-05-21T08:58:18Z

/lgtm

@paulovmr there are some flaky tests on this PR that we're turning off for now. In a follow-up PR, we need to upload cypress screenshots and videos to the CI output so we're able to revisit these tests and verify what's going on.

google-oss-prow · 2025-05-21T08:58:21Z

@caponetto: changing LGTM is restricted to collaborators

In response to this:

/lgtm

@paulovmr there are some flaky tests on this PR that we're turning off for now. In a follow-up PR, we need to upload cypress screenshots and videos to the CI output so we're able to revisit these tests and verify what's going on.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> fix cypress tests Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com>

paulovmr

/lgtm

ederign · 2025-05-21T14:03:13Z

/lgtm
/approve

google-oss-prow · 2025-05-21T14:03:21Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ederign

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~workspaces/frontend/OWNERS~~ [ederign]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

paulovmr · 2025-05-22T13:28:23Z

/unhold

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces fix cypress tests Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> Co-authored-by: Yehudit Kerido <yehudit.kerido@nokia.com> Signed-off-by: CI Bot <mkoushni@redhat.com>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces fix cypress tests Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> Co-authored-by: Yehudit Kerido <yehudit.kerido@nokia.com>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces fix cypress tests Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> Co-authored-by: Yehudit Kerido <yehudit.kerido@nokia.com> Signed-off-by: CI Bot <mkoushni@redhat.com>

feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces fix cypress tests Signed-off-by: Yehudit Kerido <yehudit.kerido@nokia.com> Co-authored-by: Yehudit Kerido <yehudit.kerido@nokia.com>

google-oss-prow bot requested review from ederign and thesuperzapper January 30, 2025 10:29

google-oss-prow bot added the size/XL label Jan 30, 2025

yehudit1987 force-pushed the fetch-workspace-list branch from 5feeff6 to 6aac5ce Compare February 2, 2025 08:24

yehudit1987 changed the title ~~Fetch workspace list~~ feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces Feb 3, 2025

yehudit1987 force-pushed the fetch-workspace-list branch from 9ba35a6 to d356044 Compare February 5, 2025 07:11

google-oss-prow bot assigned paulovmr Feb 5, 2025

yehudit1987 force-pushed the fetch-workspace-list branch 2 times, most recently from ebfda26 to 9ab07f3 Compare February 12, 2025 13:34

google-oss-prow bot added the ok-to-test label Feb 20, 2025

thesuperzapper changed the title ~~feat(ws): Notebooks 2.0 // Frontend // Fetch workspaces~~ feat(ws): stop using mocks for frontend get workspaces Feb 27, 2025

yehudit1987 marked this pull request as draft February 27, 2025 16:31

google-oss-prow bot added the do-not-merge/work-in-progress label Feb 27, 2025

thesuperzapper mentioned this pull request Mar 7, 2025

feat(ws): add http service paths to WS get on backend #213

Merged

yehudit1987 force-pushed the fetch-workspace-list branch from c15e8e5 to 6814730 Compare March 9, 2025 07:23

yehudit1987 marked this pull request as ready for review March 12, 2025 12:36

google-oss-prow bot removed the do-not-merge/work-in-progress label Mar 12, 2025

paulovmr mentioned this pull request Mar 12, 2025

[TASK] Add labels list to Workspaces details #232

Closed

4 tasks

paulovmr suggested changes Mar 12, 2025

View reviewed changes

yehudit1987 force-pushed the fetch-workspace-list branch from 61c4593 to bfade6d Compare March 17, 2025 06:34

paulovmr approved these changes Mar 17, 2025

View reviewed changes

google-oss-prow bot added the lgtm label Mar 17, 2025

ederign suggested changes Mar 19, 2025

View reviewed changes

google-oss-prow bot added the do-not-merge/hold label Mar 19, 2025

yehudit1987 force-pushed the fetch-workspace-list branch 2 times, most recently from 04ba978 to 5f22655 Compare May 20, 2025 13:25

yehudit1987 marked this pull request as ready for review May 20, 2025 13:29

google-oss-prow bot removed the do-not-merge/work-in-progress label May 20, 2025

caponetto reviewed May 20, 2025

View reviewed changes

yehudit1987 force-pushed the fetch-workspace-list branch from 5f22655 to f27f745 Compare May 21, 2025 06:26

yehudit1987 changed the title ~~feat(ws): Add workspaces tests~~ test(ws): Add workspaces tests May 21, 2025

yehudit1987 force-pushed the fetch-workspace-list branch from f27f745 to 88fab63 Compare May 21, 2025 13:27

paulovmr approved these changes May 21, 2025

View reviewed changes

google-oss-prow bot added the lgtm label May 21, 2025

ederign approved these changes May 21, 2025

View reviewed changes

google-oss-prow bot assigned ederign May 21, 2025

google-oss-prow bot added the approved label May 21, 2025

google-oss-prow bot removed the do-not-merge/hold label May 22, 2025

google-oss-prow bot merged commit 8737db2 into kubeflow:notebooks-v2 May 22, 2025
7 checks passed

github-project-automation bot moved this from Needs Triage to Done in Kubeflow Notebooks May 22, 2025

yehudit1987 deleted the fetch-workspace-list branch May 27, 2025 05:41

jiridanek mentioned this pull request Jun 2, 2025

upload cypress screenshots and videos to the CI artifacts #395

Closed

This was referenced Jun 24, 2025

Membership request for Yehudit Kerido kubeflow/internal-acls#795

Closed

Add @yehudit1987 as a member kubeflow/internal-acls#796

Merged

	const { activity, pausedTime: pauseTime, pendingRestart } = workspace;
	const { activity, pausedTime, pendingRestart } = workspace;

	{JSON.stringify(workspace.podTemplate.options.podConfig, null, 2)}
	{workspace.podTemplate.options.podConfig.current.displayName}

test(ws): Add workspaces tests #188

test(ws): Add workspaces tests #188

Uh oh!

Conversation

yehudit1987 commented Jan 30, 2025

Uh oh!

ederign commented Feb 5, 2025

Uh oh!

ederign commented Feb 5, 2025

Uh oh!

thesuperzapper commented Feb 20, 2025

Uh oh!

thesuperzapper commented Mar 7, 2025

Uh oh!

paulovmr left a comment

Choose a reason for hiding this comment

Uh oh!

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

paulovmr Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

paulovmr left a comment

Choose a reason for hiding this comment

Uh oh!

ederign left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ederign commented Mar 19, 2025

Uh oh!

caponetto left a comment

Choose a reason for hiding this comment

Uh oh!

caponetto May 20, 2025

Choose a reason for hiding this comment

Uh oh!

caponetto commented May 21, 2025

Uh oh!

google-oss-prow bot commented May 21, 2025

Uh oh!

paulovmr left a comment

Choose a reason for hiding this comment

Uh oh!

ederign commented May 21, 2025

Uh oh!

google-oss-prow bot commented May 21, 2025

Uh oh!

paulovmr commented May 22, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ederign left a comment •

edited

Loading