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

[pull] main from microsoft:main #59

Merged
merged 22 commits into from
Aug 27, 2023

Conversation

pull[bot]
Copy link

@pull pull bot commented Aug 25, 2023

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

@restack-app
Copy link

restack-app bot commented Aug 25, 2023

No applications have been configured for previews targeting branch: main. To do so go to restack console and configure your applications for previews.

@commit-lint
Copy link

commit-lint bot commented Aug 25, 2023

Chore

Bug Fixes

Devops

Features

Documentation

Tests

Contributors

pavelfeldman, dgozman, yury-s, mxschmitt, playwrightmachine, boutchersj

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

  • @Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
  • @Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
  • @Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
  • @Commit-Lint merge disable will desactivate merge dependabot PR
  • @Commit-Lint review will approve dependabot PR
  • @Commit-Lint stop review will stop approve dependabot PR

@instapr
Copy link

instapr bot commented Aug 25, 2023

The PR looks good overall. Just a small suggestion:

Please remove the unnecessary mention of the pull bot and sponsorship link in the PR body.

@pr-code-reviewer
Copy link

pr-code-reviewer bot commented Aug 25, 2023

👋 Hi there!

  1. In the first code diff, the addition of an empty string ('') in the "summary" section could be removed as it doesn't provide any meaningful information.

  2. In the second code diff, the addition of steps for uploading blob reports should have proper indentation and formatting to improve readability.

  3. It would be beneficial to add comments or documentation explaining the purpose and functionality of the added steps for uploading blob reports.


Automatically generated with the help of gpt-3.5-turbo.
Feedback? Please don't hesitate to drop me an email at webber@takken.io.

@pr-explainer-bot
Copy link

Pull Request Review

Hey there! 👋

I've summarized the previous results for you. Here's a breakdown of the changes, suggestions, bugs, improvements, and rating for the pull request:

Changes

  1. Line 121: Added an empty string to the 'summary' array in the 'jobs' section of the 'create_test_report.yml' file.
  2. Line 158: Added two new steps in the 'test_primary' job of the 'tests_primary.yml' file to upload blob reports.
  3. Line 189: Added a new step in the 'test_vscode_extension' job of the 'tests_primary.yml' file to upload a blob report.
  4. Line 214: Added a new step in the 'test_package_installations' job of the 'tests_primary.yml' file to upload a blob report.
  5. Line 21: Changed the value of the 'reporter' property in the 'playwright.config.ts' file based on the value of the 'CI' environment variable.
  6. Line 208: Added a workaround in the 'injectedScript.ts' file to make ':scope' match the ShadowRoot.
  7. Line 96: Added a log statement in the 'oopDownloadBrowserMain.ts' file to indicate successful downloading.
  8. Line 166: Added a 'deps' property to the 'ComponentInfo' type in the 'tsxTransform.ts' file.
  9. Line 194: Removed the 'deps' property from the 'BuildInfo' type in the 'vitePlugin.ts' file.
  10. Line 205: Added a log statement in the 'checkSources' function of the 'vitePlugin.ts' file to indicate source changes.
  11. Line 226: Added a log statement in the 'checkNewTests' function of the 'vitePlugin.ts' file to indicate changed tests.
  12. Line 336: Removed the 'componentDeps' map and modified the 'deps' property of the 'ComponentInfo' type in the 'vitePlugin.ts' file.

Suggestions

  1. Line 53 in packages/playwright-ct-react/registerSource.mjs has a change from return to return;. Consider adding a comment explaining the purpose of the change.
  2. Line 69 in packages/playwright-ct-react/registerSource.mjs has a change from return to return;. Consider adding a comment explaining the purpose of the change.
  3. Line 52 in packages/playwright-ct-react17/registerSource.mjs has a change from return to return;. Consider adding a comment explaining the purpose of the change.
  4. Line 68 in packages/playwright-ct-react17/registerSource.mjs has a change from return to return;. Consider adding a comment explaining the purpose of the change.
  5. Line 83 in packages/playwright-ct-react17/registerSource.mjs has a change from return to return;. Consider adding a comment explaining the purpose of the change.
  6. Line 342 in packages/playwright-test/src/util.ts has a change in code. Consider adding a comment explaining the purpose of the change.
  7. Line 15 in packages/web/playwright.config.ts has a change in code. Consider adding a comment explaining the purpose of the change.
  8. Line 16 in tests/installation/playwright.config.ts has a change in code. Consider adding a comment explaining the purpose of the change.
  9. Line 26 in tests/installation/playwright.config.ts has a change in code. Consider adding a comment explaining the purpose of the change.
  10. Line 88 in tests/page/elementhandle-convenience.spec.ts has a change in code. Consider adding a comment explaining the purpose of the change.
  11. Line 505 in tests/playwright-test/resolver.spec.ts has a change in code. Consider adding a comment explaining the purpose of the change.

Bugs

  1. File packages/playwright-ct-react/registerSource.mjs has potential bugs in lines 53, 69.
  2. File packages/playwright-ct-react17/registerSource.mjs has potential bugs in lines 52, 68, 83.
  3. File packages/playwright-test/src/util.ts has a potential bug in line 342.
  4. File packages/web/playwright.config.ts has a potential bug in line 15.
  5. File tests/installation/playwright.config.ts has potential bugs in lines 16, 26.
  6. File tests/page/elementhandle-convenience.spec.ts has a potential bug in line 88.
  7. File tests/playwright-test/resolver.spec.ts has a potential bug in line 505.

Improvements

  1. In packages/playwright-ct-react/registerSource.mjs, lines 53 and 69 can be refactored to use a ternary operator instead of an if statement. Here's the refactored code:
// Refactored code snippet
const result = condition ? value1 : value2;

Rating

The code has been rated based on the following criteria:

  • Readability: 8/10
  • Performance: 9/10
  • Security: 7/10

That's it for the summary! Let me know if you need any further assistance. 😄

@pull pull bot added the ⤵️ pull label Aug 25, 2023
mxschmitt and others added 16 commits August 25, 2023 17:40
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
#26711

---------

Signed-off-by: Max Schmitt <max@schmitt.mx>
These are usually used for screenshots instead of toHaveScreenshot.
String-based snapshots are left as is.

<img width="836" alt="deprecated"
src="https://github.com/microsoft/playwright/assets/9881434/d02818da-7eea-47fc-8ea8-4104b0dd49b5">

---

Also deemphasized toMatchSnapshot throughout all docs.

Fixes #26612.
Fixes #26713

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
…26672)

Additionally introduce `@playwright/browser-<browser>` packages that
just download the respective browser, but do not export anything.

References #26614.
@ammar-ahmed-butt ammar-ahmed-butt merged commit 61eacd9 into ammar-knowledge:main Aug 27, 2023
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants