Skip to content

Commit 540636f

Browse files
jcamargosjcamargos
committed
fix(ui): prevent reruns for pipelines without tasks
Signed-off-by: Jose Camargo <aml.camargo@hotmail.com>
1 parent 59a975f commit 540636f

4 files changed

Lines changed: 229 additions & 6 deletions

File tree

‎backend/server/services/blueprint_plan_failure_integration_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
/*
2+
Licensed to the Apache Software Foundation (ASF) under one or more
3+
contributor license agreements. See the NOTICE file distributed with
4+
this work for additional information regarding copyright ownership.
5+
The ASF licenses this file to You under the Apache License, Version 2.0
6+
(the "License"); you may not use this file except in compliance with
7+
the License. You may obtain a copy of the License at
8+
9+
http://www.apache.org/licenses/LICENSE-2.0
10+
11+
Unless required by applicable law or agreed to in writing, software
12+
distributed under the License is distributed on an "AS IS" BASIS,
13+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
See the License for the specific language governing permissions and
15+
limitations under the License.
16+
*/
17+
18+
package services
19+
20+
import (
21+
"os"
22+
"testing"
23+
"time"
24+
25+
"github.com/apache/devlake/core/errors"
26+
"github.com/apache/devlake/core/models"
27+
contextimpl "github.com/apache/devlake/impls/context"
28+
"github.com/apache/devlake/impls/dalgorm"
29+
"github.com/apache/devlake/impls/logruslog"
30+
"github.com/stretchr/testify/require"
31+
"gorm.io/driver/mysql"
32+
"gorm.io/driver/postgres"
33+
"gorm.io/gorm"
34+
)
35+
36+
// Use dedicated, disposable databases: this test creates the pipeline tables.
37+
// Unset DSNs keep ordinary unit-test runs independent of database servers.
38+
func TestBlueprintPlanFailureDatabase(t *testing.T) {
39+
for _, driver := range []string{"mysql", "postgres"} {
40+
t.Run(driver, func(t *testing.T) {
41+
env := "DEVLAKE_TEST_MYSQL_DSN"
42+
if driver == "postgres" {
43+
env = "DEVLAKE_TEST_POSTGRES_DSN"
44+
}
45+
dsn := os.Getenv(env)
46+
if dsn == "" {
47+
t.Skip("set " + env + " to a disposable database")
48+
}
49+
var dialect gorm.Dialector = mysql.Open(dsn)
50+
if driver == "postgres" {
51+
dialect = postgres.Open(dsn)
52+
}
53+
dalgorm.Init("01234567890123456789012345678901")
54+
database, err := gorm.Open(dialect, &gorm.Config{})
55+
require.NoError(t, err)
56+
sqlDB, err := database.DB()
57+
require.NoError(t, err)
58+
t.Cleanup(func() { require.NoError(t, sqlDB.Close()) })
59+
require.NoError(t, database.AutoMigrate(&models.Pipeline{}, &models.DbPipelineLabel{}, &models.Task{}))
60+
testDB := dalgorm.NewDalgorm(database)
61+
withProjectTestDatabase(t, testDB)
62+
previousRes := basicRes
63+
basicRes = contextimpl.NewDefaultBasicRes(nil, logruslog.Global, testDB)
64+
t.Cleanup(func() { basicRes = previousRes })
65+
66+
bp := &models.Blueprint{Name: "planning-failure-integration", Labels: []string{"integration"}}
67+
bp.ID = uint64(time.Now().UnixNano())
68+
t.Cleanup(func() {
69+
var ids []uint64
70+
require.NoError(t, database.Model(&models.Pipeline{}).Where("blueprint_id = ?", bp.ID).Pluck("id", &ids).Error)
71+
if len(ids) > 0 {
72+
require.NoError(t, database.Where("pipeline_id IN ?", ids).Delete(&models.DbPipelineLabel{}).Error)
73+
require.NoError(t, database.Where("blueprint_id = ?", bp.ID).Delete(&models.Pipeline{}).Error)
74+
}
75+
})
76+
planErr := errors.Default.New("source planning failed")
77+
stored, saveErr := createFailedBlueprintPipeline(bp, time.Now(), planErr)
78+
require.NoError(t, saveErr)
79+
require.NotZero(t, stored.ID)
80+
rows, count, readErr := GetDbPipelines(&PipelineQuery{BlueprintId: bp.ID, Status: models.TASK_FAILED, Label: "integration"})
81+
require.NoError(t, readErr)
82+
require.EqualValues(t, 1, count)
83+
require.Len(t, rows, 1)
84+
require.Equal(t, stored.ID, rows[0].ID)
85+
require.Equal(t, bp.Labels, rows[0].Labels)
86+
require.Empty(t, rows[0].Plan)
87+
require.Contains(t, rows[0].Message, planErr.Error())
88+
require.NotNil(t, rows[0].BeganAt)
89+
require.NotNil(t, rows[0].FinishedAt)
90+
_, pending, readErr := GetDbPipelines(&PipelineQuery{BlueprintId: bp.ID, Pending: 1})
91+
require.NoError(t, readErr)
92+
require.Zero(t, pending)
93+
var tasks int64
94+
require.NoError(t, database.Model(&models.Task{}).Where("pipeline_id = ?", stored.ID).Count(&tasks).Error)
95+
require.Zero(t, tasks)
96+
97+
// The duplicate primary key makes the label INSERT fail after the
98+
// pipeline INSERT. Neither the second pipeline nor its labels may survive.
99+
bp.Labels = []string{"duplicate", "duplicate"}
100+
failed, saveErr := createFailedBlueprintPipeline(bp, time.Now(), planErr)
101+
require.Error(t, saveErr)
102+
require.Nil(t, failed)
103+
var pipelines, labels int64
104+
require.NoError(t, database.Model(&models.Pipeline{}).Where("blueprint_id = ?", bp.ID).Count(&pipelines).Error)
105+
require.EqualValues(t, 1, pipelines)
106+
require.NoError(t, database.Model(&models.DbPipelineLabel{}).Where("name = ?", "duplicate").Count(&labels).Error)
107+
require.Zero(t, labels)
108+
})
109+
}
110+
}

‎backend/test/Readme.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,21 @@ export PKG_CONFIG_PATH=/path/to/libgit2/build
4141
export CGO_LDFLAGS='-L/path/to/libgit2/build -Wl,-rpath,/path/to/libgit2/build'
4242
make e2e-test
4343
```
44+
45+
### Planning-failure database regression test
46+
47+
`TestBlueprintPlanFailureDatabase` verifies failed-pipeline history, exclusion from
48+
the pending queue, and atomic rollback when label insertion fails. Use dedicated,
49+
disposable databases: the test creates pipeline, label, and task tables, and cleans
50+
up its own records. It does not require external data sources or a running server.
51+
52+
After generating the core and helper mocks, run from `backend/`:
53+
54+
```bash
55+
export DEVLAKE_TEST_MYSQL_DSN='merico:merico@tcp(127.0.0.1:3306)/blueprint_test?charset=utf8mb4&parseTime=True&loc=UTC'
56+
export DEVLAKE_TEST_POSTGRES_DSN='host=127.0.0.1 user=postgres dbname=blueprint_test sslmode=disable'
57+
go test -count=1 -v -run TestBlueprintPlanFailureDatabase ./server/services
58+
```
59+
60+
Each database subtest is skipped when its DSN is unset; ordinary unit tests do not
61+
require database servers.

‎config-ui/src/routes/pipeline/components/info.test.tsx‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
import { beforeEach, describe, expect, it, vi } from 'vitest';
20+
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
21+
import { ThemeProvider } from 'styled-components';
22+
23+
import { getTheme } from '@/theme/tokens';
24+
import { IPipeline, IPipelineStatus } from '@/types';
25+
26+
import { PipelineInfo } from './info';
27+
28+
const state = vi.hoisted(() => ({ data: undefined as IPipeline | undefined, rerun: vi.fn() }));
29+
vi.mock('@/hooks', () => ({ useAutoRefresh: () => ({ data: state.data }) }));
30+
vi.mock('@/api', () => ({ default: { pipeline: { rerun: state.rerun } } }));
31+
vi.mock('@/components', () => ({ Loading: () => <span>Loading</span> }));
32+
vi.mock('@/utils', async () => ({
33+
...(await import('@/utils/operator')),
34+
...(await import('@/utils/time')),
35+
}));
36+
37+
const terminalStatuses = [
38+
IPipelineStatus.COMPLETED,
39+
IPipelineStatus.PARTIAL,
40+
IPipelineStatus.FAILED,
41+
IPipelineStatus.CANCELLED,
42+
];
43+
44+
function renderPipeline(status: IPipelineStatus, totalTasks: number) {
45+
state.data = {
46+
id: 42,
47+
status,
48+
totalTasks,
49+
finishedTasks: 0,
50+
stage: 0,
51+
beganAt: null,
52+
finishedAt: null,
53+
message: 'Failed to generate pipeline plan; fix the blueprint and trigger a new run',
54+
};
55+
return render(
56+
<ThemeProvider theme={getTheme('light')}>
57+
<PipelineInfo id={42} />
58+
</ThemeProvider>,
59+
);
60+
}
61+
62+
describe('pipeline task reruns', () => {
63+
beforeEach(() => {
64+
state.rerun.mockReset().mockResolvedValue(undefined);
65+
});
66+
67+
it.each(terminalStatuses)('does not offer a task rerun for %s with no tasks', (status) => {
68+
renderPipeline(status, 0);
69+
expect(screen.queryByRole('button')).toBeNull();
70+
expect(state.rerun).not.toHaveBeenCalled();
71+
});
72+
73+
it('keeps the planning error and recovery instructions visible', () => {
74+
renderPipeline(IPipelineStatus.FAILED, 0);
75+
expect(screen.getByText('Failed to generate pipeline plan; fix the blueprint and trigger a new run')).toBeTruthy();
76+
expect(screen.getByText('0/0')).toBeTruthy();
77+
});
78+
79+
it.each(terminalStatuses)('allows existing tasks to be rerun for %s', async (status) => {
80+
renderPipeline(status, 1);
81+
fireEvent.click(screen.getByRole('button', { name: 'Rerun pipeline' }));
82+
await waitFor(() => expect(state.rerun).toHaveBeenCalledExactlyOnceWith(42));
83+
await waitFor(() =>
84+
expect(screen.getByRole('button', { name: 'Rerun pipeline' }).getAttribute('disabled')).toBeNull(),
85+
);
86+
});
87+
88+
it('does not offer reruns while the pipeline is running', () => {
89+
renderPipeline(IPipelineStatus.RUNNING, 1);
90+
expect(screen.queryByRole('button', { name: 'Rerun pipeline' })).toBeNull();
91+
});
92+
});

‎config-ui/src/routes/pipeline/components/info.tsx‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,15 @@ export const PipelineInfo = ({ id }: Props) => {
111111
{[IPipelineStatus.ACTIVE, IPipelineStatus.RUNNING, IPipelineStatus.RERUN].includes(status) && (
112112
<Button loading={operating} icon={<StopOutlined />} onClick={handleCancel} />
113113
)}
114-
{[
115-
IPipelineStatus.COMPLETED,
116-
IPipelineStatus.PARTIAL,
117-
IPipelineStatus.FAILED,
118-
IPipelineStatus.CANCELLED,
119-
].includes(status) && <Button loading={operating} icon={<RedoOutlined />} onClick={handleRerun} />}
114+
{totalTasks > 0 &&
115+
[
116+
IPipelineStatus.COMPLETED,
117+
IPipelineStatus.PARTIAL,
118+
IPipelineStatus.FAILED,
119+
IPipelineStatus.CANCELLED,
120+
].includes(status) && (
121+
<Button aria-label="Rerun pipeline" loading={operating} icon={<RedoOutlined />} onClick={handleRerun} />
122+
)}
120123
</li>
121124
</ul>
122125
{IPipelineStatus.FAILED === status && <p className="'message'">{message}</p>}

0 commit comments

Comments
 (0)