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

Make playwright tests run off of ci workflow run #681

Merged
merged 1 commit into from
Jun 20, 2023

Conversation

mickmister
Copy link
Contributor

Summary

Playwright tests are currently failing for fork PRs. This PR solves this by running the workflow alongside the ci workflow, to access variables for the Mattermost project during the workflow run.

Context https://community.mattermost.com/core/pl/buyo3qkq8tdm9yeobg6dsih4gy

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-53014

@mickmister mickmister requested a review from hanzei as a code owner June 5, 2023 01:34
@codecov-commenter
Copy link

Codecov Report

Patch and project coverage have no change.

Comparison is base (cbad10e) 15.63% compared to head (16656a8) 15.63%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #681   +/-   ##
=======================================
  Coverage   15.63%   15.63%           
=======================================
  Files          15       15           
  Lines        5518     5518           
=======================================
  Hits          863      863           
  Misses       4613     4613           
  Partials       42       42           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@hanzei hanzei left a comment

Choose a reason for hiding this comment

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

I don't see any playwright runs for this PR. Should they run for this PR?

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Jun 5, 2023
@mickmister mickmister requested a review from toninis June 5, 2023 23:26
@mickmister
Copy link
Contributor Author

@hanzei It could be because the ci / plugin-ci / delivery (pull_request) job was skipped. It's possible all of the jobs in that workflow need to run/complete?

Copy link

@toninis toninis left a comment

Choose a reason for hiding this comment

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

👍🏼

@toninis
Copy link

toninis commented Jun 6, 2023

@hanzei It could be because the ci / plugin-ci / delivery (pull_request) job was skipped. It's possible all of the jobs in that workflow need to run/complete?

This step only runs on master or main @mickmister https://github.com/mattermost/actions-workflows/blob/main/.github/workflows/plugin-ci.yml#L78

Also workflow run events need to be commited to master first before they actually run on PRs for security reasons .
Check https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_run

Note: This event will only trigger a workflow run if the workflow file is on the default branch.

@toninis
Copy link

toninis commented Jun 6, 2023

You are good to ship. I would suggest also adding an extra condition in order to check if ci workflow run was actually successful but it's fine for now

if: github.event.workflow_run.conclusion == 'success'

@toninis toninis requested a review from hanzei June 6, 2023 07:54
@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed 2: Dev Review Requires review by a core committer labels Jun 20, 2023
@hanzei hanzei added this to the v2.2.0 milestone Jun 20, 2023
@hanzei hanzei merged commit dcf1828 into master Jun 20, 2023
@hanzei hanzei deleted the playwright-workflow-run branch June 20, 2023 08:42
mickmister pushed a commit that referenced this pull request Jul 19, 2023
* [MI-2481]:Fixed issue #613 on github (#12)

* [MI-2481]:Fixed issue #681 on github

* [MI-2481]:Fixed review fixes

* [MI-2481]:Fixed review fixes

* [MI-2481]:Fixed review fixes

* [MI-2547]:Fixed review fixes for Github issue 613 and PR 626 (#14)

* [MI-2547]:Fixed review fixes for Github issue 613 and PR 626

* [MI-2547]:Fixed review fixes

* [MI-2547]:Fixed review fixes

* [MI-2582]:Fixed review comments for github issue #613 (#15)

* [MM-613]:Fixed review comments

* [MI-3072]:Converted the LHS APIs into GraphQL (#32)

* [MI-3012]: Converted user PRs API into GraphQL

* [MI-3012]: Fixed review comments

* [MI-3012]:fixed log message

* [MI-3035]: Converted the get assignment API into GraphQL

* [MI-3035]:Fixed self review comments

* [MI-3035]: Fixed review comments

* [MI-3072]:Converted review requested API to graphQL

* [MI-3072]:Combined all the graphQL queries

* [MI-3072]:Fixed CI errors

* [MI-3072]:Fixed review comments

* [MI-3072]:Fixed CI errors

* [MI-3072]:Fixed review comments

* [MI-3072]:Fixed review comments

* [MM-613]:Changed namings

* [MM-613]:Fixed review comments

* [MM-613]:Fixed review comments

* [MM-613]:Fixed panic error

* [MM-613]:Fixed review comments

* [MM-613]:Fixed lint errors

* [MM-613]:Fixed review comment

* [MM-613]:Fixed review comments

* [MM-613]:Fixed review comments
@avas27JTG avas27JTG mentioned this pull request Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants