test: Jcom/infeng 454/sign in tests

webui/react/.eslintrc.js

@@ -40,7 +40,7 @@ module.exports = {
   },
   plugins: ['import', 'jsdoc', 'react', 'react-hooks', 'sort-keys-fix'],
   root: true,
-  ignorePatterns: ['**/src/services/stream/wire.ts'],
+  ignorePatterns: ['**/src/services/stream/wire.ts', '**/src/e2e/playwright-report/**'],
   rules: {
     // Can disagree with @typescript-eslint/member-ordering.
     '@typescript-eslint/adjacent-overload-signatures': 'off',

webui/react/playwright.config.ts

@@ -17,7 +17,7 @@ export default defineConfig({
   fullyParallel: !!process.env.CI,
 
   /* https://playwright.dev/docs/test-timeouts#global-timeout */
-  globalTimeout: 3 * 60 * 1000, // 3 min
+  globalTimeout: process.env.PWDEBUG ? 0 : 3 * 60 * 1000, // 3 min unless debugging
 
   /* Folder for test artifacts such as screenshots, videos, traces, etc. */
   outputDir: './src/e2e/test-results',

webui/react/src/components/DeterminedAuth.tsx

@@ -101,12 +101,14 @@ const DeterminedAuth: React.FC<Props> = ({ canceler }: Props) => {
         ]}>
         <Input
           autoFocus
+          data-testid="username"
           placeholder="username"
           prefix={<Icon name="user" size="small" title="Username" />}
         />
       </Form.Item>
       <Form.Item name="password">
         <Input.Password
+          data-testid="password"
           placeholder="password"
           prefix={<Icon name="lock" size="small" title="Password" />}
         />
@@ -115,19 +117,24 @@ const DeterminedAuth: React.FC<Props> = ({ canceler }: Props) => {
         <p className={[css.errorMessage, css.message].join(' ')}>Incorrect username or password.</p>
       )}
       <Form.Item>
-        <Button disabled={!canSubmit} htmlType="submit" loading={isSubmitted} type="primary">
+        <Button
+          data-testid="submit"
+          disabled={!canSubmit}
+          htmlType="submit"
+          loading={isSubmitted}
+          type="primary">
           Sign In
         </Button>
       </Form.Item>
     </Form>
   );
 
   return (
-    <div className={css.base}>
+    <div className={css.base} data-test-component="detAuth">
       {loginForm}
       <p className={css.message}>
         Forgot your password, or need to manage users? Check out our&nbsp;
-        <Link external path={paths.docs('/sysadmin-basics/users.html')} popout>
+        <Link data-testid="docs" external path={paths.docs('/sysadmin-basics/users.html')} popout>
           docs
         </Link>
       </p>

webui/react/src/e2e/README.md

@@ -23,11 +23,12 @@ Deteremined AI uses [Playwright 🎭](https://playwright.dev/).
 If you don't want to use dev cluster, you can use det deploy to initiate the backend. These commands should run and pass tests on chrome:
 
 1. `det deploy local cluster-up --det-version="0.29.0" --no-gpu --master-port=8080`
-   - use whatever det-version you want here.
+   - Use whatever det-version you want here.
 2. `SERVER_ADDRESS="http://localhost:3001" npm run build --prefix webui/react`
 3. Optional if you want an experiment created for the test: `det experiment create ./examples/tutorials/mnist_pytorch/const.yaml ./examples/tutorials/mnist_pytorch/`
-4. `npm run preview --prefix webui/react` to run the preview app. Not necessary if CI=true
+4. `npm run preview --prefix webui/react` to run the preview app. Not necessary if `CI=true`.
 5. To run the tests: `PW_SERVER_ADDRESS="http://localhost:3001"  PW_USER_NAME="admin" PW_PASSWORD="" npm run e2e --prefix webui/react`
+   - Provice `-- -p=firefox` to choose one browser to run on. Full list of projects located in [playwright config](/webui/react/playwright.config.ts).
 
 ## CI
 

webui/react/src/e2e/fixtures/auth.fixture.ts

@@ -1,17 +1,41 @@
-import { Page } from '@playwright/test';
+import { expect, Page } from '@playwright/test';
+
+import { SignIn } from 'e2e/models/pages/SignIn';
 
 export class AuthFixture {
   readonly #page: Page;
-  readonly #USERNAME = process.env.PW_USER_NAME ?? '';
-  readonly #PASSWORD = process.env.PW_PASSWORD ?? '';
+  readonly #USERNAME: string;
+  readonly #PASSWORD: string;
+  readonly signInPage: SignIn;
+
   constructor(readonly page: Page) {
+    if (process.env.PW_USER_NAME === undefined) {
+      throw new Error('username must be defined');
+    }
+    if (process.env.PW_PASSWORD === undefined) {
+      throw new Error('password must be defined');
+    }
+    this.#USERNAME = process.env.PW_USER_NAME;
+    this.#PASSWORD = process.env.PW_PASSWORD;
     this.#page = page;
+    this.signInPage = new SignIn(page);
   }
 
-  async login(waitForURL: string | RegExp | ((url: URL) => boolean)): Promise<void> {
-    await this.#page.getByPlaceholder('username').fill(this.#USERNAME);
-    await this.#page.getByPlaceholder('password').fill(this.#PASSWORD);
-    await this.#page.getByRole('button', { name: 'Sign In' }).click();
+  async login(
+    waitForURL: string | RegExp | ((url: URL) => boolean) = /dashboard/,
+    {
+      username = this.#USERNAME,
+      password = this.#PASSWORD,
+    }: { username?: string; password?: string } = {},
+  ): Promise<void> {
+    const detAuth = this.signInPage.detAuth;
+    if (!(await detAuth.pwLocator.isVisible())) {
+      await this.#page.goto('/');
+      expect(detAuth.pwLocator).toBeVisible();
+    }
+    await detAuth.username.pwLocator.fill(username);
+    await detAuth.password.pwLocator.fill(password);
+    await detAuth.submit.pwLocator.click();
     await this.#page.waitForURL(waitForURL);
   }
 

webui/react/src/e2e/models/BaseComponent.ts

@@ -0,0 +1,89 @@
+import { type Locator } from '@playwright/test';
+
+import { BasePage } from './BasePage';
+
+// BasePage is the root of any tree, use `instanceof BasePage` when climbing.
+type parentTypes = BasePage | BaseComponent | BaseReactFragment;
+
+/**
+ * Returns the representation of a Component.
+ * This constructor is a base class for any component in src/components/.
+ * @param {object} obj
+ * @param {parentTypes} obj.parent - The parent used to locate this BaseComponent
+ * @param {string} obj.selector - Used as a selector uesd to locate this object
+ */
+export class BaseComponent {
+  protected _selector: string;
+  readonly _parent: parentTypes;
+  protected _locator: Locator | undefined;
+
+  constructor({ parent, selector }: { parent: parentTypes; selector: string }) {
+    this._selector = selector;
+    this._parent = parent;
+  }
+
+  /**
+   * The playwright Locator that represents this model
+   */
+  get pwLocator(): Locator {
+    if (this._locator === undefined) {
+      // Treat the locator as a readonly, but only after we've created it
+      this._locator = this._parent.pwLocator.locator(this._selector);
+    }
+    return this._locator;
+  }
+
+  get root(): BasePage {
+    let root: parentTypes = this._parent;
+    for (; !(root instanceof BasePage); root = root._parent) {
+      /* empty */
+    }
+    return root;
+  }
+}
+
+/**
+ * Returns the representation of a React Fragment.
+ * React Fragment Components are special in that they group elements, but not under a dir.
+ * Fragments cannot have selectors
+ * @param {object} obj
+ * @param {parentTypes} obj.parent - The parent used to locate this BaseComponent
+ */
+export class BaseReactFragment {
+  readonly _parent: parentTypes;
+
+  constructor({ parent }: { parent: parentTypes }) {
+    this._parent = parent;
+  }
+  /**
+   * The playwright Locator that represents this model
+   * Since this model is a fragment, we simply get the parent's locator
+   */
+  get pwLocator(): Locator {
+    return this._parent.pwLocator;
+  }
+}
+
+export type NamedComponentArgs = {
+  parent: parentTypes;
+  selector?: string;
+};
+
+/**
+ * Returns a representation of a named component.
+ * This class enforces that a `static defaultSelector` and `static url` be declared
+ * @param {object} obj
+ * @param {parentTypes} obj.parent - The parent used to locate this NamedComponent
+ * @param {string} obj.selector - Used as a selector uesd to locate this object
+ */
+export abstract class NamedComponent extends BaseComponent {
+  constructor({ parent, selector }: { parent: parentTypes; selector: string }) {
+    super({ parent, selector });
+    const requiredStaticProperties: string[] = ['defaultSelector'];
+    requiredStaticProperties.forEach((requiredProp) => {
+      if (!Object.hasOwn(this.constructor, requiredProp)) {
+        throw new Error(`A named component must declare a static ${requiredProp}!`);
+      }
+    });
+  }
+}

webui/react/src/e2e/models/BasePage.ts

@@ -0,0 +1,35 @@
+import { Locator, type Page } from '@playwright/test';
+
+/**
+ * Returns the representation of a Page.
+ * This constructor is a base class for any component in src/pages/.
+ * @param {Page} page - The '@playwright/test' Page being used by a test
+ */
+export abstract class BasePage {
+  readonly _page: Page;
+  abstract readonly url: string;
+
+  constructor(page: Page) {
+    this._page = page;
+  }
+
+  /**
+   * The playwright top-level locator
+   */
+  get pwLocator(): Locator {
+    return this._page.locator(':root');
+  }
+
+  /**
+   * Returns this so we can chain.
+   * ie. await expect(thePage.goto().theElement.loc()).toBeVisible()
+   * @param {Page} [waitForURL] - Whether for the URL to change
+   */
+  goto(waitForURL: boolean = true): BasePage {
+    this._page.goto(this.url);
+    if (waitForURL) {
+      this._page.waitForURL(this.url);
+    }
+    return this;
+  }
+}

webui/react/src/e2e/models/README.md

@@ -0,0 +1,101 @@
+# Page Model Guide
+
+Models represented in this folder are meant to be a one-to-one representations of objects from `src`.
+
+- `src/pages/SignIn.tsx` -> `src/e2e/models/pages/SignIn.ts`
+- `src/components/DeterminedAuth.tsx` -> `src/e2e/models/components/DeterminedAuth.ts`
+- `src/utils/error.ts` -> `src/e2e/models/utils/error.ts`
+
+## Why page models
+
+Playwright has [a writeup](https://playwright.dev/docs/pom) on how page models are useful, namely that they can be used as an API for interacting with pages. While this is true, there is much more to be gained from using page models.
+
+### What playwright POM is missing
+
+The playwright API approach still loosely defines each component. Relying on text can be volitile, so it would be better to use accessibility or `data-testid`. Playwright POM also serves as an API rather than a **model**.
+
+Our page models are meant to implement the precise structure of a page. This allows us to be specific in which components we represent. For exmaple, a login page might have different sign in boxes for straightforward authetication and for SSO. Or maybe a dashboard page has several tables. When we test for "a username input" or "the first row of the table", these models will provide a uniform, specific mechanism to clearly state which component we're using.
+
+In addition, playwright POM muddies their implementation with test steps executing inside the model. We want our models to represent a page that a test can perform actions on instead of acting as an interface to perform actions on that page. Here's an example.
+
+| Unclear Test Case | `jspage.createNewProject()`                            |
+| ----------------- | ------------------------------------------------------ |
+| Precise Test Case | `await page.header.createNewProject.pwLocator.click()` |
+
+The difference is suble, but in the precise exmaple, a reader will understand exactly what actions the test case is performing from a glance. This can be helpful when test cases fail and the person triaging the case isn't the same person who wrote the test. Utilities and abstarctions are still welcome, but this model pattern will elimate the need for as many!
+
+## How to use page models
+
+Simply instantiate a new page inside a test case or fixture. The page will come with properties representing the page. The `pwLocator` property gets the playwright locator which the model is representing.
+
+| Page model Autocomplete                                                 | Locator automcomplete                                             |
+| ----------------------------------------------------------------------- | ----------------------------------------------------------------- |
+| ![page model automcomplete](../docs/images/page-model-autocomplete.png) | ![locator automcomplete](../docs/images/loactor-autocomplete.png) |
+
+And here's a complete example with an `expect`:
+
+```js
+await detAuth.username.pwLocator.fill(username);
+await detAuth.password.pwLocator.fill(password);
+expect(await signInPage.detAuth.error.message.pwLocator.textContent()).toContain('Login failed');
+```
+
+## How to contribute
+
+First, if the model doesn't exist, create it. The file name and path in `src/e2e/models` with match the file name and path from it's counterpart in `src`.
+
+- Pages will inherit from `BasePage` and serve as the root of a component tree. Each page will need a `url`, and the `BasePage` class provides common properties for all pages like `goto`.
+- Named components and utilities will inherit form `NamedComponent`. This class is similar to `BaseComponent` but it enforces that a static `defaultSelector` be declared. Use this selector in the component's constructor as an alternative if `selector` isn't passed through.
+- If the component being represented returns a `React.Fragment`, inherit from the `BaseReactFragment` class
+
+All pages and components support subcomponents using instance properties. Initalize new components using instances of BaseComponent. They should support selector and parent arguments.
+
+| Argument   | Description                                                                                                                                                                                                                            |
+| ---------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `selector` | The selector passed through to playwright's `locator` method. Required if initalizing a `BaseComponent`, but optional for `NamedComponent`s.                                                                                           |
+| `parent`   | The component's parent. It could be `this` or any instance of `BaseComponent \| BasePage`. In some cases, it will be set to `this.root` if the component lives at the root of the DOM. `BasePage`s are always at the root of the tree. |
+
+Here's a simplified example using part of the `DeterminedAuth` component. Intrinsic elements are be represent with `BaseComponent`, and they can be as `parent`s for other elements. The amount of specificity is left to the author's discretion. If you're not sure, more is better to avoid conflicts with future additions. Deeper specificity will also optimize for searching through the DOM on larger pages.
+
+```js
+export class DeterminedAuth extends NamedComponent({ defaultSelector: "div[data-test-component='detAuth']"}) {
+  constructor({ selector, parent }: NamedComponentArgs) {
+    super({ parent: parent, selector: selector || DeterminedAuth.defaultSelector });
+  }
+  readonly form: BaseComponent = new BaseComponent({ parent: this, selector: "form" });
+  readonly username: BaseComponent = new BaseComponent({ parent: this.form, selector: "input[data-testid='username']" });
+  readonly password: BaseComponent = new BaseComponent({ parent: this.form, selector: "input[data-testid='password']" });
+  readonly submit: BaseComponent = new BaseComponent({ parent: this.form, selector: "button[data-testid='submit']" });
+  ...
+}
+```
+
+### Default Selectors
+
+`NamedComponent`s will have a `static defaultSelector` to use if a selector isn't provided. Since this property is static, we can use it to create elements with more details. Here's an example using an imaginary `DeterminedTable` component:
+
+```js
+  readonly userTable: DeterminedTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-testid='userTable']" });
+  readonly roleTable: DeterminedTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-testid='roleTable']" });
+```
+
+## Practices around test hooks
+
+When creating page models, you'll most likely want to author test hooks into the `src` model.
+
+| Test Hook                            | Usage                                                                |
+| ------------------------------------ | -------------------------------------------------------------------- |
+| `data-test-component='my-component'` | Belongs at the top level element wrapping the component              |
+| `data-testid='my-componentid'`       | Attributed to any _instances_ of components or any intrinsic element |
+
+Looking back to the exmaple with the imaginary `DeterminedTable`, we want to enable this pattern:
+
+```js
+  // DeterminedTable.defaultSelector = "[data-test-component='DetTable']"
+  readonly userTable: DeterminedTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-testid='userTable']" });
+  readonly roleTable: DeterminedTable = new DeterminedTable({ parent: this, selector: DeterminedTable.defaultSelector + "[data-testid='roleTable']" });
+```
+
+The component `DeterminedTable` would have `data-test-component='DetTable'` as a top level attribute, and instances would each get their own `data-testid`. This way, the static attribute and the instance attribute don't conflict with each other.
+
+Not every component needs a data-testid, but, in general, more is better. It's better to select for _"a duck named Hoffman"_ rather than "a duck" or "Hoffman".

webui/react/src/e2e/models/components/DeterminedAuth.ts

@@ -0,0 +1,39 @@
+import { BaseComponent, NamedComponent, NamedComponentArgs } from 'e2e/models/BaseComponent';
+import { ErrorComponent } from 'e2e/models/utils/error';
+
+/**
+ * Returns a representation of the DeterminedAuth component.
+ * This constructor represents the contents in src/components/DeterminedAuth.tsx.
+ * @param {object} obj
+ * @param {implementsGetLocator} obj.parent - The parent used to locate this DeterminedAuth
+ * @param {string} [obj.selector] - Used instead of `defaultSelector`
+ */
+
+export class DeterminedAuth extends NamedComponent {
+  static defaultSelector = "div[data-test-component='detAuth']";
+  constructor({ selector, parent }: NamedComponentArgs) {
+    super({ parent: parent, selector: selector || DeterminedAuth.defaultSelector });
+  }
+  readonly form: BaseComponent = new BaseComponent({ parent: this, selector: 'form' });
+  readonly username: BaseComponent = new BaseComponent({
+    parent: this.form,
+    selector: "input[data-testid='username']",
+  });
+  readonly password: BaseComponent = new BaseComponent({
+    parent: this.form,
+    selector: "input[data-testid='password']",
+  });
+  readonly submit: BaseComponent = new BaseComponent({
+    parent: this.form,
+    selector: "button[data-testid='submit']",
+  });
+  readonly docs: BaseComponent = new BaseComponent({
+    parent: this,
+    selector: "link[data-testid='docs']",
+  });
+  // TODO consdier a BaseComponents plural class
+  readonly errors: ErrorComponent = new ErrorComponent({
+    parent: this.root,
+    selector: ErrorComponent.defaultSelector + ErrorComponent.selectorTopRight,
+  });
+}