voidmatcha commented on code in PR #5262:
URL: https://github.com/apache/zeppelin/pull/5262#discussion_r3342586588
##########
zeppelin-web-angular/e2e/models/folder-rename-page.ts:
##########
@@ -77,12 +67,11 @@ export class FolderRenamePage extends BasePage {
}
async enterNewName(name: string): Promise<void> {
- await this.renameInput.fill(name);
+ await this.fillAndVerifyInput(this.renameInput, name);
Review Comment:
pressSequentially reproduced the same race. Angular's FormControl
initializes after any key/value input, so every input method hits the same
reset window. Routed this and the other modal-input specs through
BasePage.fillAndVerifyInput instead.
##########
zeppelin-web-angular/e2e/models/node-list-page.ts:
##########
@@ -41,15 +41,12 @@ export class NodeListPage extends BasePage {
await this.createNewNoteButton.click();
}
- private getNoteByName(noteName: string): Locator {
- return this.page.locator('nz-tree-node').filter({ hasText: noteName
}).first();
+ noteLinkByName(noteName: string): Locator {
+ return this.nodeListContainer.getByRole('link', { name: noteName, exact:
true });
Review Comment:
Master's substring hasText + .first() non-deterministically picked one of
several TestNotebook_* nodes coexisting in the shared workspace.
getByRole('link', { name, exact: true }) is exact-match and the
Playwright-recommended locator.
##########
zeppelin-web-angular/e2e/global.setup.ts:
##########
@@ -0,0 +1,45 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as fs from 'fs';
+import * as path from 'path';
+import { test as setup, expect } from '@playwright/test';
+import { LoginTestUtil } from './models/login-page.util';
+import { performLoginIfRequired, waitForZeppelinReady } from './utils';
+
+// Resolved against the Playwright project rootDir (zeppelin-web-angular/).
+// Must match the `storageState` value declared for browser projects in
playwright.config.js.
+export const STORAGE_STATE = path.join('playwright', '.auth', 'user.json');
+
+setup('authenticate', async ({ page }) => {
+ fs.mkdirSync(path.dirname(STORAGE_STATE), { recursive: true });
+
+ const isShiroEnabled = await LoginTestUtil.isShiroEnabled();
+ if (!isShiroEnabled) {
+ // Auth variant disabled — write an empty storage state so dependent
projects load,
+ // then exit. This keeps the setup-project pattern uniform across CI
matrix variants.
+ await page.context().storageState({ path: STORAGE_STATE });
+ return;
+ }
+
+ await page.goto('/');
+ await waitForZeppelinReady(page);
+
+ await performLoginIfRequired(page);
+
+ // Verify we are authenticated. Don't rely on performLoginIfRequired's
return value —
+ // it returns false both for "no work to do" and "login attempt failed".
+ await expect(page.locator('zeppelin-login')).toBeHidden({ timeout: 30000 });
+ await expect(page.getByRole('heading', { name: 'Welcome to Zeppelin!'
})).toBeVisible({ timeout: 30000 });
+
+ await page.context().storageState({ path: STORAGE_STATE });
+});
Review Comment:
This setup project is the core change in the PR. The 29 specs that
previously called login in their own beforeEach raced on the shared session
cookie under parallel workers. Consolidating to a single login + storageState
removes that race.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
