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

Replace Saucelabs with Playwright #1793

Merged
merged 8 commits into from
Dec 29, 2021
Merged

Replace Saucelabs with Playwright #1793

merged 8 commits into from
Dec 29, 2021

Conversation

jaylinski
Copy link
Member

Also reorganized test-folders.

@jaylinski jaylinski self-assigned this Dec 4, 2021
@jaylinski jaylinski force-pushed the browser-tests branch 7 times, most recently from fcbc1ff to 6fc5324 Compare December 4, 2021 20:21
Copy link
Collaborator

@nknapp nknapp left a comment

Choose a reason for hiding this comment

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

There is a lot of cleanup in this one commit, along with the actual changes. That makes it hard to track the actual changes.

It would be easier to review if there were separate commits in thie PR: One for "renaming and moving stuff", one for "replacing saucelabs with playwright".

Not in this PR anymore...

I left two remarks. Please think about them. Other than that I think the changes are good and the renaming and moving files is beneficial for understanding the project.

We should try to move away from grunt...

tests/server.js Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
First part of reorganizing and cleaning up test-folders.
Second part of reorganizing and cleaning up test-folders.
@jaylinski jaylinski force-pushed the browser-tests branch 2 times, most recently from 122427b to 9c3125d Compare December 22, 2021 19:56
Third part of reorganizing and cleaning up test-folders.
Also removed it from published files, since the script doesn't seem to be used.

Fourth part of reorganizing and cleaning up test-folders.
Also reorganized npm scripts.
@jaylinski jaylinski force-pushed the browser-tests branch 4 times, most recently from 4a25bca to 1a67625 Compare December 22, 2021 23:17
@jaylinski
Copy link
Member Author

It would be easier to review if there were separate commits in thie PR

@nknapp I split the changes in multiple commits, if you want to take another look.

tests/server.js Outdated Show resolved Hide resolved
Gruntfile.js Show resolved Hide resolved
@nknapp nknapp self-requested a review December 23, 2021 23:10
Copy link
Collaborator

@nknapp nknapp left a comment

Choose a reason for hiding this comment

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

custom-server should be replaced with grunt-connect (see my comment there). We can replace it later, when we know better in which direction we are going

@jaylinski
Copy link
Member Author

custom-server should be replaced with grunt-connect (see my comment there). We can replace it later, when we know better in which direction we are going

@nknapp Done in 315c1ad.

@jaylinski jaylinski merged commit 78e7e28 into 4.x Dec 29, 2021
@jaylinski jaylinski deleted the browser-tests branch December 29, 2021 20:35
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.

2 participants