Skip to content

Commit

Permalink
fix: unexpected status on workspace startup (#1109)
Browse files Browse the repository at this point in the history
Signed-off-by: Oleksii Kurinnyi <okurinny@redhat.com>
  • Loading branch information
akurinnoy authored May 13, 2024
1 parent a010b15 commit 7c64a6c
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,58 @@ describe('Starting steps, opening an editor', () => {
});
});

test('workspace is FAILING', async () => {
test('workspace status change from STOPPING to RUNNING', async () => {
isAvailableEndpointMock.mockResolvedValue(true);

const store = new FakeStoreBuilder()
.withDevWorkspaces({
workspaces: [
new DevWorkspaceBuilder()
.withName(workspaceName)
.withNamespace(namespace)
.withStatus({ phase: 'STOPPING' })
.build(),
],
})
.build();
const { reRenderComponent } = renderComponent(store);

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

expect(mockOnError).not.toHaveBeenCalledWith();
expect(mockOnNextStep).not.toHaveBeenCalled();
expect(mockOnRestart).not.toHaveBeenCalled();

const storeNext = new FakeStoreBuilder()
.withDevWorkspaces({
workspaces: [
new DevWorkspaceBuilder()
.withName(workspaceName)
.withNamespace(namespace)
.withStatus({ phase: 'RUNNING', mainUrl: 'main-url' })
.build(),
],
})
.build();
reRenderComponent(storeNext);

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

expect(mockLocationReplace).toHaveBeenCalledWith('main-url');

expect(mockOnNextStep).toHaveBeenCalled();
expect(mockOnError).not.toHaveBeenCalled();
expect(mockOnRestart).not.toHaveBeenCalled();
});

test('workspace is FAILED', async () => {
const store = new FakeStoreBuilder()
.withDevWorkspaces({
workspaces: [
new DevWorkspaceBuilder()
.withName(workspaceName)
.withNamespace(namespace)
.withStatus({ phase: 'FAILING' })
.withStatus({ phase: 'FAILED' })
.build(),
],
})
Expand All @@ -171,7 +215,7 @@ describe('Starting steps, opening an editor', () => {
// should report the error
const expectAlertItem = expect.objectContaining({
title: 'Failed to open the workspace',
children: 'The workspace status changed unexpectedly to "Failing".',
children: 'The workspace status changed unexpectedly to "Failed".',
actionCallbacks: [
expect.objectContaining({
title: 'Restart',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ import { TimeLimit } from '@/components/WorkspaceProgress/TimeLimit';
import { WorkspaceParams } from '@/Routes/routes';
import { isAvailableEndpoint } from '@/services/helpers/api-ping';
import { findTargetWorkspace } from '@/services/helpers/factoryFlow/findTargetWorkspace';
import { AlertItem, LoaderTab } from '@/services/helpers/types';
import { AlertItem, DevWorkspaceStatus, LoaderTab } from '@/services/helpers/types';
import { Workspace, WorkspaceAdapter } from '@/services/workspace-adapter';
import { AppState } from '@/store';
import { selectApplications } from '@/store/ClusterInfo/selectors';
Expand Down Expand Up @@ -132,9 +132,18 @@ class StartingStepOpenWorkspace extends ProgressStep<Props, State> {
}

if (!workspace.isRunning) {
throw new Error(
workspace.error || `The workspace status changed unexpectedly to "${workspace.status}".`,
);
if (
workspace.status === DevWorkspaceStatus.FAILED ||
workspace.status === DevWorkspaceStatus.TERMINATING
) {
throw new Error(
workspace.error || `The workspace status changed unexpectedly to "${workspace.status}".`,
);
}

// it's possible for the workspace to be restarted before the IDE URL is available
// in this case we need to wait for all conditions to be met
return false;
}

if (!workspace.ideUrl) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -398,22 +398,8 @@ describe('Starting steps, starting a workspace', () => {

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

// should report the error
const expectAlertItem = expect.objectContaining({
title: 'Failed to open the workspace',
children: 'The workspace status changed unexpectedly to "Stopping".',
actionCallbacks: [
expect.objectContaining({
title: 'Restart',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Restart with default devfile',
callback: expect.any(Function),
}),
],
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));
// should not report any error
expect(mockOnError).not.toHaveBeenCalled();

// should not start the workspace
expect(mockStartWorkspace).not.toHaveBeenCalled();
Expand Down Expand Up @@ -518,22 +504,8 @@ describe('Starting steps, starting a workspace', () => {

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

// should report the error
const expectAlertItem = expect.objectContaining({
title: 'Failed to open the workspace',
children: 'The workspace status changed unexpectedly to "Failing".',
actionCallbacks: [
expect.objectContaining({
title: 'Restart',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Restart with default devfile',
callback: expect.any(Function),
}),
],
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));
// should not report any error
expect(mockOnError).not.toHaveBeenCalled();

expect(mockStartWorkspace).not.toHaveBeenCalled();
expect(mockOnNextStep).not.toHaveBeenCalled();
Expand Down Expand Up @@ -617,22 +589,8 @@ describe('Starting steps, starting a workspace', () => {

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

// should report the error
const expectAlertItem = expect.objectContaining({
title: 'Failed to open the workspace',
children: 'The workspace status changed unexpectedly to "Failing".',
actionCallbacks: [
expect.objectContaining({
title: 'Restart',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Restart with default devfile',
callback: expect.any(Function),
}),
],
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));
// should not report any error
expect(mockOnError).not.toHaveBeenCalled();

expect(mockOnNextStep).not.toHaveBeenCalled();
expect(mockOnRestart).not.toHaveBeenCalled();
Expand All @@ -655,22 +613,8 @@ describe('Starting steps, starting a workspace', () => {

await jest.advanceTimersByTimeAsync(MIN_STEP_DURATION_MS);

// should report the error
const expectAlertItem = expect.objectContaining({
title: 'Failed to open the workspace',
children: 'The workspace status changed unexpectedly to "Stopping".',
actionCallbacks: [
expect.objectContaining({
title: 'Restart',
callback: expect.any(Function),
}),
expect.objectContaining({
title: 'Restart with default devfile',
callback: expect.any(Function),
}),
],
});
await waitFor(() => expect(mockOnError).toHaveBeenCalledWith(expectAlertItem));
// should not report any error
expect(mockOnError).not.toHaveBeenCalled();

expect(mockOnNextStep).not.toHaveBeenCalled();
expect(mockOnRestart).not.toHaveBeenCalled();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,7 @@ class StartingStepStartWorkspace extends ProgressStep<Props, State> {
}

if (
workspaceStatusIs(
workspace,
DevWorkspaceStatus.TERMINATING,
DevWorkspaceStatus.STOPPING,
DevWorkspaceStatus.FAILING,
) ||
workspaceStatusIs(workspace, DevWorkspaceStatus.TERMINATING) ||
(this.state.shouldStart === false &&
workspaceStatusIs(workspace, DevWorkspaceStatus.STOPPED, DevWorkspaceStatus.FAILED))
) {
Expand Down

0 comments on commit 7c64a6c

Please sign in to comment.