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

chore(docs): Rework CI docs #31988

Merged
merged 11 commits into from
Aug 20, 2024
Merged

chore(docs): Rework CI docs #31988

merged 11 commits into from
Aug 20, 2024

Conversation

Skn0tt
Copy link
Member

@Skn0tt Skn0tt commented Aug 2, 2024

This PR moves around some of our CI docs. It moves the GitHub actions docs from ci-intro.md to ci.md, reduces ci-intro.md to be an introduction, adds a mention of Sharding to the best practices, and adds a section on --only-changed called "Fail-Fast". Each of those changes is a separate commit, to make this a little easier to review. If we find any of those to commits to be contentious, i'll pull them out into individual PRs.

While rolling this to playwright.dev, we'll also make the following changes to its sidebar:

  • move the ci.md document from the "Integrations" section to the "Playwright Test" section
  • make "Best Practices" the last item of the "Getting Started" section

@Skn0tt Skn0tt self-assigned this Aug 2, 2024
docs/src/ci.md Outdated Show resolved Hide resolved
docs/src/ci-intro.md Outdated Show resolved Hide resolved
docs/src/ci.md Outdated Show resolved Hide resolved
docs/src/ci-intro.md Outdated Show resolved Hide resolved
with:
name: playwright-report
path: playwright-report/
retention-days: 30
```

### On push/pull_request
If this doesn't make sense to you, give GitHub's [Understanding GitHub Actions](https://docs.github.com/en/actions/learn-github-actions/understanding-github-actions) guide a read. Looking at the list of steps in `jobs.test.steps`, you can see that the workflow performs these steps:
Copy link
Contributor

Choose a reason for hiding this comment

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

same comment as below regarding wording if this doesnt make sense

Copy link
Member Author

Choose a reason for hiding this comment

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

done in 34ccc7a

@@ -25,22 +23,18 @@ When [installing Playwright](./intro.md) using the [VS Code extension](./getting

Playwright tests can be ran on any CI provider. In this section we will cover running tests on GitHub using GitHub actions. If you would like to see how to configure other CI providers check out our detailed doc on Continuous Integration.

To add a [GitHub Actions](https://docs.github.com/en/actions) file first create `.github/workflows` folder and inside it add a `playwright.yml` file containing the example code below so that your tests will run on each push and pull request for the main/master branch.

#### You will learn
* langs: python, java, csharp
Copy link
Contributor

Choose a reason for hiding this comment

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

just double check this makes sense for all langs except js or it should also have js?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this is intended. There's a separate "You will learn" list for JS that has different contents in https://github.com/microsoft/playwright/pull/31988/files#diff-49b43d48c93c3dc274299a0604e3a7c8ea1ed9891a230f3125e9992867d2ed3fL13-L14.

docs/src/best-practices-js.md Outdated Show resolved Hide resolved
docs/src/ci-intro.md Outdated Show resolved Hide resolved
docs/src/ci-intro.md Outdated Show resolved Hide resolved
docs/src/ci-intro.md Show resolved Hide resolved
docs/src/ci.md Outdated
## CI configurations

### GitHub Actions

The [Command line tools](./browsers#install-system-dependencies) can be used to install all operating system dependencies on GitHub Actions.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand who this sentence is addressed to under GitHub Actions title. Previously the user was redirected to our comprehensive getting started guide which is where they should start. If they know that stuff, they likely now how to install browsers and deps already. I'd revert Check out our [GitHub Actions](ci-intro.md) guide here or maybe just drop this paragraph.

Copy link
Member Author

Choose a reason for hiding this comment

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

This PR moves the comprehensive github actions guide into here, so ci-intro.md is just an intro. I agree that the paragraph reads weird though, max was saying the same in #31988 (comment). I'll move the paragraph above the heading and reword.

Skn0tt and others added 3 commits August 9, 2024 11:55
Co-authored-by: Yury Semikhatsky <yurys@chromium.org>
Signed-off-by: Simon Knott <info@simonknott.de>
Copy link
Contributor

github-actions bot commented Aug 9, 2024

Test results for "tests 1"

5 flaky ⚠️ [firefox-page] › page/page-event-request.spec.ts:169:3 › should return response body when Cross-Origin-Opener-Policy is set
⚠️ [chromium-library] › library/trace-viewer.spec.ts:969:1 › should not crash with broken locator
⚠️ [playwright-test] › ui-mode-test-watch.spec.ts:145:5 › should watch all
⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:165:5 › should update tracing network live
⚠️ [webkit-library] › library/video.spec.ts:797:5 › screencast › should work with video+trace

29658 passed, 711 skipped
✔️✔️✔️

Merge workflow run.

@Skn0tt Skn0tt merged commit 244761a into microsoft:main Aug 20, 2024
5 checks passed
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.

4 participants