Skip to content

Conversation

sergiolms
Copy link
Contributor

Continuation from #3374

This PR has only the Playwright code.
Keeping the previous PR closed with the wdio implementation so we keep its history tracked.

Playwright tests can be run with yarn test:e2e

@sergiolms sergiolms self-assigned this Aug 22, 2024
@sergiolms sergiolms requested a review from sergeibbb August 26, 2024 11:28
@axosoft-ramint
Copy link
Contributor

Since I'm not very familiar with how playwright works, may I ask what settings it is using when run? For example, if I have the graph opening in editor by default, or I have hidden/disabled GitLens Inspect in the sidebar, do those tests fail?

@sergiolms
Copy link
Contributor Author

sergiolms commented Aug 28, 2024

@axosoft-ramint

Since I'm not very familiar with how playwright works, may I ask what settings it is using when run? For example, if I have the graph opening in editor by default, or I have hidden/disabled GitLens Inspect in the sidebar, do those tests fail?

On the Playwright config I set a setup.ts file that downloads a stable and insiders version of VSCode. Later in the same playwright.config.ts file under projects you can specify where should tests run, and I set the stable version of VSCode there.

playwright.conf.ts running tests

Playwright creates a .vscode-test folder in GitLens' repo and downloads them there. In later runs, it reuses those (it also checks if they need to update).
image

Since it's Playwright's VSCode, it won't run with your VSCode settings. Moreover, GitLens is installed in each run, so consider every new test a fresh start with default settings.

image

For that same reason, if you change a setting from a test, it shouldn't persist on later tests, I assume.
I guess that for testing settings we would need to do something like "Close settings page, open settings again, assert changed option persists" on the same test run.

I hope this helped understand a little bit more what's under the hood. 😄

@sergiolms sergiolms force-pushed the chore/add_e2e_tests_playwright branch from 4f48687 to cff16af Compare August 28, 2024 10:24
@axosoft-ramint
Copy link
Contributor

When I try it with yarn test:e2e, all my tests are failing. Here are the logs:

$ yarn test:e2e
yarn run v1.22.21
$ playwright test -c tests/e2e/playwright.config.ts
✔ Validated version: insiders
✔ Found at https://update.code.visualstudio.com/latest/win32-x64-archive/insider?released=true
✔ Downloaded VS Code into C:\Projects\GitLens\vscode-gitlens\.vscode-test\vscode-win32-x64-archive-insiders
✔ Validated version: 1.92.2
✔ Found at https://update.code.visualstudio.com/1.92.2/win32-x64-archive/stable?released=true
✔ Downloaded VS Code into C:\Projects\GitLens\vscode-gitlens\.vscode-test\vscode-win32-x64-archive-1.92.2

Running 3 tests using 1 worker

  ✘  1 …d_palette.test.ts:4:6 › Test GitLens Command Palette commands › should open commit graph with the command (5.2s)- Resolving version...
✔ Validated version: 1.92.2
✔ Found existing install in C:\Projects\GitLens\vscode-gitlens\.vscode-test\vscode-win32-x64-archive-1.92.2
  ✘  2 …_install.test.ts:4:6 › Test GitLens installation › should display GitLens Welcome page after installation (3.1s)- Resolving version...
✔ Validated version: 1.92.2
✔ Found existing install in C:\Projects\GitLens\vscode-gitlens\.vscode-test\vscode-win32-x64-archive-1.92.2
  ✘  3 ….test.ts:9:6 › Test GitLens installation › should contain GitLens & GitLens Inspect icons in activity bar (2.7s)- Resolving version...
✔ Validated version: 1.92.2
✔ Found existing install in C:\Projects\GitLens\vscode-gitlens\.vscode-test\vscode-win32-x64-archive-1.92.2


  1) [VSCode stable] › specs\command_palette.test.ts:4:6 › Test GitLens Command Palette commands › should open commit graph with the command

    Error: electronApplication.firstWindow: Target page, context or browser has been closed

       at specs\baseTest.ts:43

      41 |              });
      42 |
    > 43 |              const page = await electronApp.firstWindow();
         |                                             ^
      44 |              await page.context().tracing.start({
      45 |                      screenshots: true,
      46 |                      snapshots: true,

        at Object.page (C:\Projects\GitLens\vscode-gitlens\tests\e2e\specs\baseTest.ts:43:34)

  2) [VSCode stable] › specs\gitlens_install.test.ts:4:6 › Test GitLens installation › should display GitLens Welcome page after installation

    Error: electronApplication.firstWindow: Target page, context or browser has been closed

       at specs\baseTest.ts:43

      41 |              });
      42 |
    > 43 |              const page = await electronApp.firstWindow();
         |                                             ^
      44 |              await page.context().tracing.start({
      45 |                      screenshots: true,
      46 |                      snapshots: true,

        at Object.page (C:\Projects\GitLens\vscode-gitlens\tests\e2e\specs\baseTest.ts:43:34)

  3) [VSCode stable] › specs\gitlens_install.test.ts:9:6 › Test GitLens installation › should contain GitLens & GitLens Inspect icons in activity bar

    Error: electronApplication.firstWindow: Target page, context or browser has been closed

       at specs\baseTest.ts:43

      41 |              });
      42 |
    > 43 |              const page = await electronApp.firstWindow();
         |                                             ^
      44 |              await page.context().tracing.start({
      45 |                      screenshots: true,
      46 |                      snapshots: true,

        at Object.page (C:\Projects\GitLens\vscode-gitlens\tests\e2e\specs\baseTest.ts:43:34)

  3 failed
    [VSCode stable] › specs\command_palette.test.ts:4:6 › Test GitLens Command Palette commands › should open commit graph with the command
    [VSCode stable] › specs\gitlens_install.test.ts:4:6 › Test GitLens installation › should display GitLens Welcome page after installation
    [VSCode stable] › specs\gitlens_install.test.ts:9:6 › Test GitLens installation › should contain GitLens & GitLens Inspect icons in activity bar
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

@sergiolms
Copy link
Contributor Author

sergiolms commented Aug 29, 2024

@axosoft-ramint

When I try it with yarn test:e2e, all my tests are failing.

  1) [VSCode stable] › specs\command_palette.test.ts:4:6 › Test GitLens Command Palette commands › should open commit graph with the command

    Error: electronApplication.firstWindow: Target page, context or browser has been closed

       at specs\baseTest.ts:43

      41 |              });
      42 |
    > 43 |              const page = await electronApp.firstWindow();
         |                                             ^

From what I see it looks like the electron app (VSCode in this case) is closing early so when it tries to fetch the firstWindow() it errors.
I'm not sure if this is related to the test itself at all. I think it could be a timeout problem. I set 30 seconds, I'm going to increase it to 1 min instead and see if it works for you. Please if you have time could you run the tests again?
And if it errors for the same reason, could you increase the value to something ridiculous like 3 or 5 min? That should be more than enough for tests to run. If it keeps happening then I'm afraid something's definitely happening with the electron instance.

Apart from that, I'm removing an await from a keyboard press, as it could be causing also that it resolves after the application has closed, causing the next page call to error (it's not happening to me, but I've seen some edge cases).
Keyboard inputs shouldn't be awaited, only selectors. So even though I'm not sure if this will solve it, it's definitely an improvement. 🤷‍♂️

Hopefully these changes will help

@axosoft-ramint axosoft-ramint self-assigned this Aug 29, 2024
Copy link
Contributor

@axosoft-ramint axosoft-ramint left a comment

Choose a reason for hiding this comment

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

@sergiolms All tests are passing now - LGTM! Thanks for fixing it!

Copy link
Member

@eamodio eamodio left a comment

Choose a reason for hiding this comment

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

Looks great!

@eamodio eamodio merged commit 803db7f into main Sep 6, 2024
2 checks passed
@eamodio eamodio deleted the chore/add_e2e_tests_playwright branch October 14, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants