Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integration Tests #11186

Merged
merged 8 commits into from
Sep 27, 2024
Merged

Integration Tests #11186

merged 8 commits into from
Sep 27, 2024

Conversation

farmaazon
Copy link
Contributor

Pull Request Description

A stub for integration tests to be run locally; part of #8487

To run tests, you need to:

  1. Build IDE package: ./run ide build
  2. set ENSO_TEST_USER and ENSO_TEST_USER_PASSWORD to some working credentials (I used my personal account, but there will be also test user soon)
  3. run corepack pnpm -r --filter enso exec playwright test

The tests are run with a separate projects directory set up in tmpdir, so any local workspace dir is not affected.

The only test so far just checks if it's possible to log in and create a new project.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • [ ] Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    TypeScript,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • [ ] Unit tests have been written where possible.

@farmaazon farmaazon added CI: No changelog needed Do not require a changelog entry for this PR. g-electron GUI: electron application specific -gui labels Sep 26, 2024
@farmaazon farmaazon self-assigned this Sep 26, 2024
.slice(1)
.filter(
option =>
option !== '--no-sandbox' &&
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: consider using a regex instead

await expect(page.getByRole('textbox', { name: 'password' })).toBeVisible()
if (process.env.ENSO_TEST_USER == null || process.env.ENSO_TEST_USER_PASSWORD == null) {
throw Error(
'Cannot log in; ENSO_TEST_USER and ENSO_TEST_USER_PASSWORD env variables are not provided',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: personal preference but i like it when code (ENSO_TEST_USER) is surrounded by backticks

await page.keyboard.press('Enter')

// Accept terms screen
await expect(page.getByText('I agree')).toHaveCount(2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suppose this is sufficient for now. but we will probably want to extract dashboard and/or GUI actions to a shared package

@@ -0,0 +1,13 @@
/** @file A test for basic flow of the application: open project and see if nodes appear. */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit re: file name - for test files named after actions, i name them like methods - so present (present simple) tense instead of continuous tense (createNewProject)

@farmaazon farmaazon added CI: Ready to merge This PR is eligible for automatic merge and removed CI: Ready to merge This PR is eligible for automatic merge labels Sep 26, 2024
args
.slice(1)
// Omitting $ in --inspect and --remote-debugging-port is intentional.
.filter(option => !/^--no-sandbox$|^--inspect|^--remote-debugging-port/.test(option))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider inspect\b and port\b. not perfect but might as well

@farmaazon farmaazon added the CI: Ready to merge This PR is eligible for automatic merge label Sep 27, 2024
@mergify mergify bot merged commit e6b904d into develop Sep 27, 2024
37 of 39 checks passed
@mergify mergify bot deleted the wip/farmaazon/integration-tests branch September 27, 2024 15:11
@JaroslavTulach
Copy link
Member

I am at 96fa2ee and I tried:

enso$ ./run ide build
enso$ corepack pnpm -r --filter enso exec playwright test
Running 1 test using 1 worker

  ✘  1 electronTest.ts:13:3 › Create new project (287ms)


  1) electronTest.ts:13:3 › Create new project ─────────────────────────────────────────────────────

    Error: Process failed to launch!

  1 failed
    electronTest.ts:13:3 › Create new project ──────────────────────────────────────────────────────
/enso/app/ide-desktop/client:
 ERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL  Command failed with exit code 1: playwright test

I guess either the ./run command or the instructions need to be updated to make sure playwright is installed on my computer.

@somebody1234
Copy link
Contributor

maybe you're missing pnpm i?

@farmaazon
Copy link
Contributor Author

I guess either the ./run command or the instructions need to be updated to make sure playwright is installed on my computer.

No, playwright is installed by pnpm (in this case: as part of ./run ide build.

The issue is very strange, could you run it with DEBUG="pw:browser log:"?

@JaroslavTulach
Copy link
Member

./run ide build

With 73abe90 I can:

enso$ export DEBUG="pw:browser log:"
enso$ ./run ide build
...
INFO main_internal: enso_build_cli: close
INFO enso_build_cli: Successfully ending.

and then corepack pnpm -r --filter enso exec playwright test really opens browser and tries to log in. That fails, but things seem to be on right track.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge g-electron GUI: electron application specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants