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

feat(allure-playwright): initial implementation for allure-playwright #297

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

pavelfeldman
Copy link
Contributor

@pavelfeldman pavelfeldman commented Jul 17, 2021

Fixes #296, microsoft/playwright#7034

This is initial implementation for Playwright reporter. I already booked allure-playwright npm package for it, please let me know who I should transfer ownership over it to. I did not push that package.

Please let me know what you think about it and I can add more tests for it.

@CLAassistant
Copy link

CLAassistant commented Jul 17, 2021

CLA assistant check
All committers have signed the CLA.

@just-boris
Copy link
Contributor

just-boris commented Jul 17, 2021

Hello!

Thanks for the PR! There are known build failures, will be fixed soon #298
I will also try to run it locally and play with the new reporter in my demo project

@pavelfeldman pavelfeldman force-pushed the initial_impl branch 2 times, most recently from 47164e3 to 40b7125 Compare July 19, 2021 23:25
@pavelfeldman
Copy link
Contributor Author

I added a couple more tests.

@baev
Copy link
Member

baev commented Jul 20, 2021

I already booked allure-playwright npm package for it, please let me know who I should transfer ownership over it to. I did not push that package.

https://www.npmjs.com/~baev

Can we also add language and framework labels for each test?

@pavelfeldman
Copy link
Contributor Author

https://www.npmjs.com/~baev

Done.

Can we also add language and framework labels for each test?

Done.

@pavelfeldman
Copy link
Contributor Author

Tell me if I should kick myself out of the owners for allure-playwright or you want someone from Playwright team there.

@pavelfeldman
Copy link
Contributor Author

Playwright v1.13 is out and I published this package as experimental-allure-playwright to let users try it out.

@mpuertao
Copy link

@baev good day. When will the changes be approved?

@baev
Copy link
Member

baev commented Jul 30, 2021

@pavelfeldman could you please fix the build?

@pavelfeldman
Copy link
Contributor Author

You probably want to merge #298, otherwise current master is broken:

allure-jasmine: test/label.test.ts(10,14): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/label.test.ts(11,16): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(10,15): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(11,17): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(25,15): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(26,17): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(40,15): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(41,17): error TS2339: Property 'xit' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(55,15): error TS2339: Property 'xdescribe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/status.test.ts(56,17): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/step.test.ts(10,15): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/step.test.ts(11,17): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/step.test.ts(35,15): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/step.test.ts(36,17): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/step.test.ts(74,15): error TS2339: Property 'describe' does not exist on type 'JasmineTestEnv'.
allure-jasmine: test/step.test.ts(75,17): error TS2339: Property 'it' does not exist on type 'JasmineTestEnv'.

@pavelfeldman
Copy link
Contributor Author

@baev ^^

@pavelfeldman pavelfeldman force-pushed the initial_impl branch 2 times, most recently from 478081e to 638e433 Compare August 2, 2021 22:31
@pavelfeldman
Copy link
Contributor Author

pavelfeldman commented Aug 2, 2021

It should build/test now, but mocha coverage fails for me locally...
^^ @baev

@baev baev added the type:new feature New feature or request label Aug 3, 2021
@baev
Copy link
Member

baev commented Aug 3, 2021

It should build/test now, but mocha coverage fails for me locally...

Fixed coverage in master, please rebase (also prettier/eslint config was added, please consider aligning code style as well)

@pavelfeldman
Copy link
Contributor Author

Your eslint is quite unusual, but should be aligned now.

@baev
Copy link
Member

baev commented Aug 6, 2021

@pavelfeldman

> [email protected] test /home/runner/work/allure-js/allure-js/packages/allure-playwright
> playwright test -c test

·undefined
F···

  1) screenshot-on-failure.spec.ts:19:1 › should capture screenshot on failure =====================

    Error: expect(received).toEqual(expected) // deep equality

    - Expected  - 7
    + Received  + 1

    - Array [
    -   Object {
    -     "file": "/home/runner/work/allure-js/allure-js/packages/allure-playwright/test-results/screenshot-on-failure-should-capture-screenshot-on-failure/test-results/a-test-name/test-failed-1.png",
    -     "name": "screenshot",
    -     "type": "image/png",
    -   },
    - ]
    + Array []

      42 |     },
      43 |   );
    > 44 |   expect(result).toEqual([
         |                  ^
      45 |     {
      46 |       name: "screenshot",
      47 |       type: "image/png",

        at /home/runner/work/allure-js/allure-js/packages/allure-playwright/test/screenshot-on-failure.spec.ts:44:18
        at processTicksAndRejections (internal/process/task_queues.js:95:5)
        at WorkerRunner._runTestWithBeforeHooks (/home/runner/work/allure-js/allure-js/node_modules/@playwright/test/lib/test/workerRunner.js:390:7)


  1 failed
    screenshot-on-failure.spec.ts:19:1 › should capture screenshot on failure ======================
  4 passed (8s)

@pavelfeldman
Copy link
Contributor Author

@baev that test was running full Playwright and we need to install browser deps and/or include GH action for that. That's one line in yaml, but I removed the test for now to unblock it as is.

@baev baev merged commit bbdc670 into allure-framework:master Aug 6, 2021
@baev
Copy link
Member

baev commented Aug 6, 2021

@pavelfeldman @just-boris thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add allure-playwright
5 participants