Skip to content

Commit 161b5d6

Browse files
sanketpathaksanketpathak
committed
Address review comments
1 parent 8c6c040 commit 161b5d6

7 files changed

Lines changed: 55 additions & 108 deletions

File tree

‎workspaces/orchestrator/app-config.yaml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,10 @@ orchestrator:
161161
# Uncomment to set the content length limit for the requests. Defaults to 102400 bytes (100kb)
162162
# contentLengthLimit: 10mb
163163
# Uncomment and configure to use the log viewer
164-
# workflowLogProvider:
165-
# loki:
166-
# baseUrl: http://localhost:3100
164+
workflowLogProvider:
165+
loki:
166+
baseUrl: http://localhost:3100
167+
token: secret-token
167168
# Uncomment if using a custom log stream selector
168169
# logStreamSelectors:
169170
# - label: 'selector'

‎workspaces/orchestrator/e2e-tests/orchestrator.test.ts‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -113,9 +113,7 @@ test.describe('Orchestrator workflow runs', () => {
113113
);
114114
});
115115

116-
test('Verify workflow runs table', async ({
117-
browser: _browser,
118-
}, testInfo) => {
116+
test('Verify workflow runs table', async ({}, testInfo) => {
119117
await runAccessibilityTests(sharedPage, testInfo);
120118
await orchestratorHelper.verifyTableHeadingAndRows([
121119
translations.table.headers.name,
@@ -125,28 +123,26 @@ test.describe('Orchestrator workflow runs', () => {
125123
translations.table.headers.successRatio,
126124
'Actions',
127125
]);
128-
129-
await orchestratorHelper.searchInputPlaceholder('Hello World workflow');
126+
const workflowName = 'Hello World workflow';
127+
await orchestratorHelper.searchInputPlaceholder(workflowName);
130128
await expect(
131129
sharedPage
132-
.getByRole('row', { name: 'Hello World workflow' })
130+
.getByRole('row', { name: workflowName })
133131
.getByRole('button', {
134132
name: translations.table.actions.run,
135133
exact: true,
136134
})
137135
.first(),
138136
).toBeVisible();
139137
await expect(
140-
sharedPage.getByRole('row', { name: 'Hello World workflow' }),
138+
sharedPage.getByRole('row', { name: workflowName }),
141139
).toContainText(translations.workflow.status.available);
142140
await expect(
143-
sharedPage.getByRole('row', { name: 'Hello World workflow' }),
141+
sharedPage.getByRole('row', { name: workflowName }),
144142
).toContainText('1.0');
145143
});
146144

147-
test('Run Test Object Type Support in ui:props workflow', async ({
148-
browser: _browser,
149-
}) => {
145+
test('Run Test Object Type Support in ui:props workflow', async () => {
150146
const workflowName = 'Test Object Type Support in ui:props';
151147
const workflowInputs = {
152148
name: 'test-name',
@@ -163,9 +159,7 @@ test.describe('Orchestrator workflow runs', () => {
163159
await orchestrator.verifyUiPropsWorkflowRunVariables(workflowInputs);
164160
});
165161

166-
test('Greeting workflow execution and workflow tab validation', async ({
167-
browser: _browser,
168-
}) => {
162+
test('Greeting workflow execution and workflow tab validation', async () => {
169163
const workflowName = 'Greeting workflow';
170164

171165
await orchestrator.runGreetingWorkflow(workflowName);
@@ -176,31 +170,29 @@ test.describe('Orchestrator workflow runs', () => {
176170
await orchestrator.searchWorkflow(workflowName);
177171
await orchestrator.validateGreetingWorkflowTableRow(workflowName);
178172
await sharedPage
179-
.getByRole('row', { name: 'Greeting workflow' })
173+
.getByRole('row', { name: workflowName })
180174
.getByRole('link', { name: workflowName })
181175
.first()
182176
.click();
183177
await orchestrator.validateWorkflowDetails(workflowName);
184178
});
185179

186-
test('Greeting workflow re-run and run details validation', async ({
187-
browser: _browser,
188-
}) => {
180+
test('Greeting workflow re-run and run details validation', async () => {
189181
const workflowName = 'Greeting workflow';
190182

191183
await orchestrator.runGreetingWorkflow(workflowName);
192184
await orchestrator.reRunGreetingWorkflow();
193185
await orchestrator.verifyWorkflowRunDetails();
194186
});
195187

196-
test('Sample Retry Test', async ({ browser: _browser }) => {
188+
test('Sample Retry Test', async () => {
197189
const workflowName = 'Sample Retry Test';
198190

199191
await orchestrator.runSampleRetryTest(workflowName);
200192
await orchestrator.verifySampleRetryTest();
201193
});
202194

203-
test('Add workflow run by entity', async ({ browser: _browser }) => {
195+
test('Add workflow run by entity', async () => {
204196
await orchestrator.navigateToCatalog();
205197
await expect(
206198
sharedPage
@@ -235,7 +227,7 @@ test.describe('Orchestrator workflow runs', () => {
235227
);
236228
});
237229

238-
test('Verify all runs tab', async ({ browser: _browser }, testInfo) => {
230+
test('Verify all runs tab', async ({}, testInfo) => {
239231
await orchestrator.navigateToWorkflowRunTab(
240232
translations.page.tabs.workflows,
241233
);
@@ -263,9 +255,7 @@ test.describe('Orchestrator workflow runs', () => {
263255
});
264256

265257
// Remove the fixme with the fix of bug https://redhat.atlassian.net/browse/RHDHBUGS-3401
266-
test.fixme('All runs tab workflow details validation', async ({
267-
browser: _browser,
268-
}) => {
258+
test.fixme('All runs tab workflow details validation', async () => {
269259
await sharedPage
270260
.getByRole('link', { name: 'Hello World workflow' })
271261
.first()

‎workspaces/orchestrator/e2e-tests/pages/orchestrator.ts‎

Lines changed: 30 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,6 @@ export class Orchestrator {
7979
.waitFor({ state: 'visible', timeout: 30_000 });
8080
}
8181

82-
async verifyKeyValueRowElements(rowTitle: string, rowValue: string) {
83-
const rowLocator = this.page.locator('.v5-MuiTableRow-root');
84-
await expect(rowLocator.filter({ hasText: rowTitle })).toContainText(
85-
rowValue,
86-
);
87-
}
88-
8982
async searchWorkflow(workflowName: string) {
9083
await this.orchestratorHelper.searchInputPlaceholder(workflowName);
9184
await expect(
@@ -150,50 +143,33 @@ export class Orchestrator {
150143
`${this.translations.workflow.fields.runStatus} ${this.translations.table.status.completed}`,
151144
),
152145
).toBeVisible({ timeout: 60_000 });
153-
// TODO: Remove the if statement for japanese language tests with solving bug https://redhat.atlassian.net/browse/RHDHBUGS-3406
154-
if (this.locale === 'ja') {
155-
await expect(
156-
this.page.getByText(
157-
`${this.translations.run.results}${this.translations.table.actions.run}`,
158-
),
159-
).toBeVisible();
160-
} else {
146+
147+
// TODO: Remove the ja branch when bug https://redhat.atlassian.net/browse/RHDHBUGS-3406 is fixed
148+
const resultsText =
149+
this.locale === 'ja'
150+
? `${this.translations.run.results}${this.translations.table.actions.run}`
151+
: `${this.translations.run.results}${this.translations.run.status.completed}`;
152+
153+
const instanceDetailTexts = [
154+
resultsText,
155+
`${this.translations.workflow.fields.workflow}${displayWorkflowName}`,
156+
`${this.translations.workflow.fields.workflowStatus} ${this.translations.workflow.status.available}`,
157+
];
158+
for (const text of instanceDetailTexts) {
159+
await expect(this.page.getByText(text)).toBeVisible();
160+
}
161+
162+
const instanceDetailHeadings = [
163+
this.translations.workflow.fields.workflowId,
164+
this.translations.workflow.fields.duration,
165+
this.translations.workflow.fields.started,
166+
this.translations.workflow.fields.description,
167+
];
168+
for (const heading of instanceDetailHeadings) {
161169
await expect(
162-
this.page.getByText(
163-
`${this.translations.run.results}${this.translations.run.status.completed}`,
164-
),
170+
this.page.getByRole('heading', { name: heading }),
165171
).toBeVisible();
166172
}
167-
await expect(
168-
this.page.getByText(
169-
`${this.translations.workflow.fields.workflow}${displayWorkflowName}`,
170-
),
171-
).toBeVisible();
172-
await expect(
173-
this.page.getByText(
174-
`${this.translations.workflow.fields.workflowStatus} ${this.translations.workflow.status.available}`,
175-
),
176-
).toBeVisible();
177-
await expect(
178-
this.page.getByRole('heading', {
179-
name: this.translations.workflow.fields.workflowId,
180-
}),
181-
).toBeVisible();
182-
await expect(
183-
this.page.getByRole('heading', {
184-
name: this.translations.workflow.fields.duration,
185-
}),
186-
).toBeVisible();
187-
await expect(
188-
this.page.getByRole('heading', {
189-
name: this.translations.workflow.fields.started,
190-
}),
191-
).toBeVisible();
192-
await expect(
193-
this.page.getByRole('heading', {
194-
name: this.translations.workflow.fields.description,
195-
}),
196-
).toBeVisible();
197173
}
198174

199175
private formatRunVariablesStringField(
@@ -260,7 +236,7 @@ export class Orchestrator {
260236
await this.runGreetingWorkflowFromExecuteForm();
261237
}
262238

263-
async reRunGreetingWorkflow(language = 'english') {
239+
async reRunGreetingWorkflow(language = 'English') {
264240
await expect(
265241
this.page.getByText(this.translations.workflow.buttons.runAgain),
266242
).toBeVisible();
@@ -321,7 +297,7 @@ export class Orchestrator {
321297
name: this.translations.workflow.inputSchemaDescription,
322298
})
323299
.click();
324-
expect(
300+
await expect(
325301
this.page.getByRole('tooltip', {
326302
name: this.translations.workflow.inputSchemaDescription,
327303
}),
@@ -355,17 +331,17 @@ export class Orchestrator {
355331
exact: true,
356332
}),
357333
).toBeVisible();
358-
expect(
334+
await expect(
359335
this.page.getByText(this.translations.workflow.statsSuccess, {
360336
exact: true,
361337
}),
362338
).toBeVisible();
363-
expect(
339+
await expect(
364340
this.page.getByText(this.translations.workflow.statsFailed, {
365341
exact: true,
366342
}),
367343
).toBeVisible();
368-
expect(
344+
await expect(
369345
this.page.getByText(this.translations.workflow.runSuccess, {
370346
exact: true,
371347
}),
@@ -380,7 +356,7 @@ export class Orchestrator {
380356
name: this.translations.workflow.successRatioDescription,
381357
})
382358
.click();
383-
expect(
359+
await expect(
384360
this.page.getByRole('tooltip', {
385361
name: this.translations.workflow.successRatioDescription,
386362
}),

‎workspaces/orchestrator/e2e-tests/utils/helper.ts‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,9 @@ export class OrchestratorHelper {
4747
}
4848

4949
async searchInputPlaceholder(searchTerm: string) {
50-
await this.page.fill(
51-
`input[placeholder="${this.translations.table.filters.placeholder}"]`,
52-
searchTerm,
53-
);
50+
await this.page
51+
.getByPlaceholder(this.translations.table.filters.placeholder)
52+
.fill(searchTerm);
5453
}
5554

5655
async verifyHeading(heading: string | RegExp, timeout: number = 20000) {
@@ -76,19 +75,8 @@ export class OrchestratorHelper {
7675
await barButton.click();
7776
}
7877

79-
async clickLink(options: string | { href: string } | { ariaLabel: string }) {
80-
let linkLocator: Locator;
81-
82-
if (typeof options === 'string') {
83-
linkLocator = this.page.locator('a').filter({ hasText: options }).first();
84-
} else if ('href' in options) {
85-
linkLocator = this.page.locator(`a[href*="${options.href}"]`).first();
86-
} else {
87-
linkLocator = this.page
88-
.locator(`div[aria-label='${options.ariaLabel}'] a`)
89-
.first();
90-
}
91-
78+
async clickLink(options: string | RegExp) {
79+
const linkLocator = this.page.getByRole('link', { name: options });
9280
await linkLocator.waitFor({ state: 'visible' });
9381
await linkLocator.click();
9482
}

‎workspaces/orchestrator/package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
"build:api-reports:only": "backstage-repo-tools api-reports --allow-all-warnings -o ae-wrong-input-file-type --validate-release-tags --exclude 'plugins/orchestrator-swf-editor-envelope'",
1818
"build:knip-reports": "backstage-repo-tools knip-reports",
1919
"clean": "backstage-cli repo clean",
20-
"test": "backstage-cli repo test --detectOpenHandles",
21-
"test:all": "backstage-cli repo test --coverage --detectOpenHandles",
20+
"test": "backstage-cli repo test",
21+
"test:all": "backstage-cli repo test --coverage",
2222
"test:legacy": "APP_MODE=legacy playwright test",
2323
"test:nfs": "APP_MODE=nfs playwright test",
2424
"test:e2e:ci": "yarn test:legacy && yarn test:nfs",

‎workspaces/orchestrator/playwright.config.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@ export default defineConfig({
4747

4848
retries: process.env.CI ? 2 : 0,
4949

50-
workers: process.env.CI ? 2 : 1,
51-
5250
reporter: [
5351
['html', { open: 'never', outputFolder: `e2e-test-report-${appMode}` }],
5452
],

‎workspaces/orchestrator/plugins/orchestrator-backend-module-loki/src/module.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,6 @@ export const orchestratorModuleLoki = createBackendModule({
3636
workflowLogs: workflowLogsExtensionEndpoint,
3737
},
3838
async init({ config, logger, workflowLogs }) {
39-
if (!config.has('orchestrator.workflowLogProvider.loki')) {
40-
logger.info(
41-
'Loki workflow log provider is not configured, skipping module initialization',
42-
);
43-
return;
44-
}
4539
logger.info('Initialize the Loki orchestrator backend module');
4640
workflowLogs.addWorkflowLogProvider(LokiProvider.fromConfig(config));
4741
},

0 commit comments

Comments
 (0)