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

playwright: upgrade dependency #12141

Conversation

ndoschek
Copy link
Contributor

@ndoschek ndoschek commented Feb 2, 2023

What it does

Update playwright to the latest version (and open up version range) as there was a breaking change regarding playwright's Page object.

Signed-off-by: Nina Doschek ndoschek@eclipsesource.com

Remark: There is currently one flaky test case, see also #12063.

How to test

  1. confirm that playwright tests pass via test:playwright

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added dependencies pull requests that update a dependency file playwright issues related to playwright tests labels Feb 2, 2023
@vince-fugnitto
Copy link
Member

vince-fugnitto commented Feb 2, 2023

I created a review for @playwright/test/1.30.0 given a pull-request from a fork cannot automatically create the review during CI 👍

Edit: the dependency has been approved.

marcdumais-work added a commit that referenced this pull request Feb 6, 2023
Suring CI we run `dash-licenses` to check that the project's 3PP dependencies are
approved by the Eclipse foundation. When a PR originates from the main repo, a
token is available that permits running the tool in "automated review mode",
which opens IP tickets automatically towards the Eclipse Foundation. When a
PR originates from elsewhere, that token is not available and so we fall-back
to reporting issues in the CI log.

Until now, the "-project" option of `dash-licenses` was only thought useful
un "automated review" mode, but it turns-out there is a rare case where we
benefit providing this information all the time: when a 3PP dependency was
narrowly approved, for use in Eclipse Theia only. here is one such dependency:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/2734

The dependency above is part of a recent PR from an outside contributor
(non-committer), that originated from a fork:

#12141

So far, for PRs originating from a fork, we would not provide the project when
running `dash-licenses` and so such dependency are incorrectly flagged as
unapproved:

https://github.com/eclipse-theia/theia/actions/runs/4075784869/jobs/7077702838#step:5:186

This commit provides the project all the time, so that such dependencies will
be correctly assessed, based on what's approved for our project, even for
non-committer contributors.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work added a commit that referenced this pull request Feb 6, 2023
During CI we run `dash-licenses` to check that the project's 3PP dependencies are
approved by the Eclipse foundation. When a PR originates from the main repo, a
token is available that permits running the tool in "automated review mode",
which opens IP tickets automatically towards the Eclipse Foundation. When a
PR originates from elsewhere, that token is not available and so we fall-back
to reporting issues in the CI log.

Until now, the "-project" option of `dash-licenses` was only thought useful
un "automated review" mode, but it turns-out there is a rare case where we
benefit providing this information all the time: when a 3PP dependency was
narrowly approved, for use in Eclipse Theia only. here is one such dependency:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/2734

The dependency above is part of a recent PR from an outside contributor
(non-committer), that originated from a fork:

#12141

So far, for PRs originating from a fork, we would not provide the project when
running `dash-licenses` and so such dependency are incorrectly flagged as
unapproved:

https://github.com/eclipse-theia/theia/actions/runs/4075784869/jobs/7077702838#step:5:186

This commit provides the project all the time, so that such dependencies will
be correctly assessed, based on what's approved for our project, even for
non-committer contributors.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work added a commit that referenced this pull request Feb 6, 2023
During CI we run `dash-licenses` to check that the project's 3PP dependencies are
approved by the Eclipse foundation. When a PR originates from the main repo, a
token is available that permits running the tool in "automated review mode",
which opens IP tickets automatically towards the Eclipse Foundation. When a
PR originates from elsewhere, that token is not available and so we fall-back
to reporting issues in the CI log.

Until now, the "-project" option of `dash-licenses` was only thought useful
in "automated review" mode, but it turns-out there is a rare case where we
benefit providing this information all the time: when a 3PP dependency was
narrowly approved, for use in Eclipse Theia only. Here is one such dependency:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/2734

The dependency above is part of a recent PR from an outside contributor
(non-committer), that originated from a fork:

#12141

So far, for PRs originating from a fork, we would not provide the project when
running `dash-licenses` and so such dependency are incorrectly flagged as
unapproved:

https://github.com/eclipse-theia/theia/actions/runs/4075784869/jobs/7077702838#step:5:186

This commit provides the project all the time, so that such dependencies will
be correctly assessed, based on what's approved for our project, even for
non-committer contributors.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work added a commit that referenced this pull request Feb 6, 2023
During CI we run `dash-licenses` to check that the project's 3PP dependencies are
approved by the Eclipse foundation. When a PR originates from the main repo, a
token is available that permits running the tool in "automated review mode",
which opens IP tickets automatically towards the Eclipse Foundation. When a
PR originates from elsewhere, that token is not available and so we fall-back
to reporting issues in the CI log.

Until now, the "-project" option of `dash-licenses` was only thought useful
in "automated review" mode, but it turns-out there is a rare case where we
benefit providing this information all the time: when a 3PP dependency was
narrowly approved, for use in Eclipse Theia only. Here is one such dependency:

https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/2734

The dependency above is part of a recent PR from an outside contributor
(non-committer), that originated from a fork:

#12141

So far, for PRs originating from a fork, we would not provide the project when
running `dash-licenses` and so such dependency are incorrectly flagged as
unapproved:

https://github.com/eclipse-theia/theia/actions/runs/4075784869/jobs/7077702838#step:5:186

This commit provides the project all the time, so that such dependencies will
be correctly assessed, based on what's approved for our project, even for
non-committer contributors.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
@vince-fugnitto
Copy link
Member

@ndoschek do you mind rebasing to pick up #12151? It should fix the issue where playwright-core@1.30.0 was being marked as non-compliant in our "3PP License Check".

Update `playwright` to the latest version (and open up version range) as there was a breaking change regarding playwright's Page object.

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com>
@ndoschek
Copy link
Contributor Author

ndoschek commented Feb 7, 2023

Thank you! I just rebased to latest master

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@vince-fugnitto vince-fugnitto merged commit a60077e into eclipse-theia:master Feb 8, 2023
@ndoschek ndoschek deleted the ndoschek/update-playwright branch February 9, 2023 08:53
@vince-fugnitto vince-fugnitto added this to the 1.35.0 milestone Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies pull requests that update a dependency file playwright issues related to playwright tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants