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.

Sign up for GitHub

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

Jump to bottom

Configure integration tests with Playwright #126

Merged
merged 1 commit into from
May 2, 2023
Merged

Configure integration tests with Playwright #126

merged 1 commit into from
May 2, 2023

Conversation

PatrickTasse
Copy link
Contributor

@PatrickTasse PatrickTasse commented Apr 18, 2023 •
edited
Loading

Add and configure Playwright in vscode-trace-extension package.

Add extension-test.spec.ts with a few simple tests.

Ignore and exclude test files and folders from build and version control.

Partially fixes Issue #34

@PatrickTasse PatrickTasse force-pushed the playwright branch 5 times, most recently from 07de916 to b8d8d9b Compare April 24, 2023 13:51
@PatrickTasse PatrickTasse marked this pull request as draft April 24, 2023 15:47
@PatrickTasse PatrickTasse marked this pull request as ready for review April 27, 2023 13:56
@PatrickTasse PatrickTasse force-pushed the playwright branch 2 times, most recently from e01c5b0 to e8df6c7 Compare April 27, 2023 17:50
@marco-miller
Copy link
Contributor

Should this PR and its commit (message) link to Issue #34 (ready for review)?

@marco-miller marco-miller self-requested a review May 1, 2023 14:53
marco-miller
marco-miller requested changes May 1, 2023
View reviewed changes
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

Thanks!

marco-miller
marco-miller requested changes May 1, 2023
View reviewed changes
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

I get this sole failing test locally during the yarn playwright test step in README; maybe I'm missing something compared to your own local setup(?):

image

@marco-miller marco-miller requested a review from bhufmann May 1, 2023 16:03
@bhufmann
Copy link
Collaborator

bhufmann commented May 1, 2023

I get this sole failing test locally during the yarn playwright test step in README; maybe I'm missing something compared to your own local setup(?):

image

Yes, I get the same error, which is weird because it was working fine last week. @PatrickTasse could you please look into this?

bhufmann
bhufmann approved these changes May 1, 2023
View reviewed changes
Copy link
Collaborator

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution of the playwright tests. I think it's great addition. I think we can work in merging this as-is (after resolving the remaining comments). However, we should plan follow-ups simplify the steps for users to run setup and tests.

bhufmann
bhufmann requested changes May 1, 2023
View reviewed changes
Copy link
Collaborator

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

I missed to request changes before and accidentally press approved.

@PatrickTasse
Copy link
Contributor Author

I get this sole failing test locally during the yarn playwright test step in README; maybe I'm missing something compared to your own local setup(?):
image

Yes, I get the same error, which is weird because it was working fine last week. @PatrickTasse could you please look into this?

Is it occasionally or consistently failing for you? There seems to be some unexplained flakiness to the tests, but it's not failing for me at all today.

@marco-miller
Copy link
Contributor

Is it occasionally or consistently failing for you? There seems to be some unexplained flakiness to the tests, but it's not failing for me at all today.

Consistently, for me locally so far.

marco-miller
marco-miller requested changes May 1, 2023
View reviewed changes
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

In case this comment is applicable; thanks.

bhufmann
bhufmann reviewed May 1, 2023
View reviewed changes
marco-miller
marco-miller requested changes May 1, 2023
View reviewed changes
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

Nit left. And I still get that failing test despite setting --retries=3 or retrying manually. But we could consider merging this initial PR if, say, at least two other setups pass all tests (in my opinion; more welcome).

@marco-miller
Copy link
Contributor

(...) I still get that failing test despite setting --retries=3 or retrying manually. But we could consider merging this initial PR if, say, at least two other setups pass all tests (in my opinion; more welcome).

All tests are passing for me locally today.

@marco-miller
Copy link
Contributor

(...) I still get that failing test despite setting --retries=3 or retrying manually. But we could consider merging this initial PR if, say, at least two other setups pass all tests (in my opinion; more welcome).

All tests are passing for me locally today.

Well, only the first time I ran them earlier. That failing test now consistently fails again.

@PatrickTasse
Configure integration tests with Playwright
33d70b1 
Add and configure Playwright in vscode-trace-extension package.

Add extension-test.spec.ts with a few simple tests.

Ignore and exclude test files and folders from build and version
control.

Partially fixes Issue eclipse-cdt-cloud#34

Signed-off-by: Patrick Tasse <[email protected]>
marco-miller
marco-miller approved these changes May 2, 2023
View reviewed changes
Copy link
Contributor

@marco-miller marco-miller left a comment

Choose a reason for hiding this comment

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

LGTM now, thanks!

bhufmann
bhufmann approved these changes May 2, 2023
View reviewed changes
Copy link
Collaborator

@bhufmann bhufmann left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks for contributing the Playwright tests. We can build on that for more automated tests.

vscode-trace-extension/src/test/extension-test.spec.ts
test('Open Trace from Explorer', async ({ page }) => {
await page.getByRole('treeitem', { name: '202-bug-hunt' }).locator('a').click();
await page.getByRole('treeitem', { name: 'cat-kernel' }).locator('a').click({ button: 'right' });
await page.getByRole('menuitem', { name: 'Open with Trace Viewer' }).hover();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line fixes the intermittent failures when running it on my computer. Let's leep it in mind when implementing more tests in later commits.

@PatrickTasse PatrickTasse merged commit e054dc8 into eclipse-cdt-cloud:master May 2, 2023
@PatrickTasse PatrickTasse deleted the playwright branch May 2, 2023 16:11
@PatrickTasse PatrickTasse mentioned this pull request May 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bhufmann bhufmann bhufmann approved these changes

@marco-miller marco-miller marco-miller approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PatrickTasse @marco-miller @bhufmann