New API #76

mmarkelov · 2020-03-19T10:04:55Z

Thinking about rewriting jest-playwright with new API:
Using jest-playwright --parallel command is kind of hack, so I think about replacing it.

To run tests for specified browsers in you config you should do:
jest-playwright.config.js:

module.exports = {
    browsers: [ 'firefox', 'webkit', 'chromium'],
}

In tests:

describe('Google', () => {
    beforeAll(async () => {
       // It will navigate all browsers to url
        await page.goto('https://whatismybrowser.com/')
    })

    it('should display "google" text on page', async () => {
        const titles = await page.title()
        // Thinking about more clear and nice way to handle it
        titles.forEach(title => expect(title).toBe('What browser am I using? Is my browser up to date?'))
       // It will just return result for all browsers in array
        const [firefox, webkit, chrome] = await page.$eval('.string-major', el => el.innerHTML)

        expect(firefox).toContain('Firefox')
        expect(webkit).toContain('Safari')
        expect(chrome).toContain('Chrome')

    })
})

Let me know what do you think about it?

mxschmitt · 2020-03-19T14:15:21Z

Thats a nice idea! So you are just replacing the implementation for multiple browsers as far as I've seen right?
Not fully sure about the const [firefox, webkit, chrome] part, maybe replace it with an object, because then it's easier to handle which key is associated with with browser by using object deconstruction.
Also your solution would mean in the end, that the user always have to check for N custom browsers which might be not so good for all the users and from the clean coding perspective.
But for every solution there are some disadvantages and when we compare this solution with the previous one, then this one is much nicer from my perspective.

mmarkelov · 2020-03-19T14:25:53Z

@mxschmitt yeah! That is what I'm thinking about.
Replacing const [firefox, webkit, chrome] is a good idea, I suppose.
Yes, it will be not the cleanest way to do tests. But I think it is better way to tests multiple browsers.
Of cause this is not perfect, because of this:

    it('should display "google" text on page', async () => {
        const titles = await page.title()
        // That is not the best way
        titles.forEach(title => expect(title).toBe('What browser am I using? Is my browser up to date?'))
       // It will just return result for all browsers in array
        const {firefox, webkit, chrome} = await page.$eval('.string-major', el => el.innerHTML)
    })

I'm trying to improve it, but not sure that it will be possible to simplify it more

mxschmitt · 2020-03-19T14:31:38Z

@mxschmitt yeah! That is what I'm thinking about.
Replacing const [firefox, webkit, chrome] is a good idea, I suppose.
Yes, it will be not the cleanest way to do tests. But I think it is better way to tests multiple browsers.
Of cause this is not perfect, because of this:
    it('should display "google" text on page', async () => {
        const titles = await page.title()
        // That is not the best way
        titles.forEach(title => expect(title).toBe('What browser am I using? Is my browser up to date?'))
       // It will just return result for all browsers in array
        const {firefox, webkit, chrome} = await page.$eval('.string-major', el => el.innerHTML)
    })
I'm trying to improve it, but not sure that it will be possible to simplify it more

One Idea would be to force the user that he needs to make

["chromium", "firefox", "webkit"].forEach(browserName => {
  const page = jestPlaywright.getBrowser(browserName)
  describe(`Testing browser ${browserName}`, () => {
    it("should do XYZ", async() => {
      
    })
  })
})

idk. Then reporting would also be kinda better afaik.

mmarkelov · 2020-03-20T14:30:55Z

Some new thoughts here. I just added helper proxy to simplify test process:

describe('Google', () => {
  beforeAll(async () => {
    // It will navigate all browsers to url
    await page.goto('https://whatismybrowser.com/');
  });

  it('should display "google" text on page', async () => {
    const titles = await page.title();
    // It will expect toBe for all browsers

    // Passed tests
    expectAllBrowsers(titles).toBe('What browser am I using? Is my browser up to date?')

    // Failed tests
    const myBrowser = await page.$eval(
      '.string-major',
      el => el.innerHTML,
    );

    expectAllBrowsers(myBrowser).toContain('Chrome')
  });
});

I just think about nice error message here. For now it will be:

 FAIL  ./index.test.js (5.459s)
  Google
    ✕ should display "google" text on page (104ms)

  ● Google › should display "google" text on page

    expect(received).toContain(expected) // indexOf

    Expected substring: "Chrome"
    Received string:    "·······························
                                    <a href=\"/detect/what-version-of-firefox-do-i-have?utm_source=whatismybrowsercom&amp;utm_medium=internal&amp;utm_campaign=homepage&amp;utm_content=simple-major\">Firefox 73 on macOS (Catalina)</a>·······························
                    "

      18 |     );
      19 | 
    > 20 |     expectAllBrowsers(myBrowser).toContain('Chrome')
         |                                  ^
      21 |   });
      22 | });
      23 | 

      at browsers.forEach.browser (node_modules/jest-playwright-preset/lib/PlaywrightEnvironment.js:112:67)
          at Array.forEach (<anonymous>)
      at Proxy.toContain (node_modules/jest-playwright-preset/lib/PlaywrightEnvironment.js:107:34)
      at Object.it (index.test.js:20:34)

Failed test for firefox

mxschmitt · 2020-03-21T21:14:26Z

cool! let's maybe ask the users who are actually using this feature what they think about that before we merge?

mmarkelov · 2020-03-22T09:14:05Z

@mxschmitt yeah! I just want to support jest-playwright commands for some time. But I want to replace it with this one soon

…/jest-playwright into New-API � Conflicts: � package-lock.json � package.json � src/PlaywrightEnvironment.ts

…into New-API � Conflicts: � package-lock.json � src/PlaywrightEnvironment.ts

…/jest-playwright into New-API � Conflicts: � src/PlaywrightEnvironment.ts

github-actions · 2020-04-09T16:19:53Z

Pull Request Test Coverage Report for Build 47062a08620269e9ec26ff327fa6d35919c56745-PR-76

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 94.03%

Totals
Change from base Build 0be92af709e115b393e1653f103545baebfb9750:	0.0%
Covered Lines:	75
Relevant Lines:	76

💛 - Coveralls

mxschmitt · 2020-04-09T16:58:58Z

src/PlaywrightEnvironment.ts

@@ -1,6 +1,9 @@
 /* eslint-disable no-console */
 import NodeEnvironment from 'jest-environment-node'
 import { Config as JestConfig } from '@jest/types'
+import Expect = jest.Expect
+import playwright, { Browser, BrowserContext, Page } from 'playwright'


please import types from playwright-core

…/jest-playwright into New-API

mmarkelov · 2020-04-16T15:03:18Z

Some updates here. Just tried to use it with jest-circus runner, it is ok. But for some reason it needs to run with forceExit and detectOpenHandles options. Still working on it

jest --forceExit --detectOpenHandles

…into New-API � Conflicts: � src/PlaywrightEnvironment.ts

mmarkelov · 2020-04-17T07:41:28Z

Some updates. I got passed tests on local machine. But got some issues with webkit. But for now it's quite working solution. I think it's better to use tests like so:

import path from 'path'

jest.setTimeout(10000)

describe('Example HTML file', () => {
  it('should detect the heading "Example" on page', async () => {
    await page.goto(`file:${path.join(__dirname, 'example.html')}`)
    const browser = await page.$eval('h1', (el) => el.textContent)
    expectAllBrowsers(browser).toBe('Example')
  })
})

Also I'm thinking about adding expectChromium and others for running expect function for specified browser. IDK how it will be relate to #96

@mxschmitt your thoughts?)

mxschmitt · 2020-04-17T10:00:23Z

jest-playwright.config.js

@@ -0,0 +1,4 @@
+module.exports = {
+  USE_NEW_API: true,


why no camel case here?

@mxschmitt I think that snake style will show that it's experimental feature

mxschmitt · 2020-04-17T10:00:41Z

src/PlaywrightEnvironment.ts

@@ -1,16 +1,29 @@
 /* eslint-disable no-console */
 import NodeEnvironment from 'jest-environment-node'
 import { Config as JestConfig } from '@jest/types'
+import Expect = jest.Expect


Suggested change

import Expect = jest.Expect

mxschmitt · 2020-04-17T10:01:23Z

src/PlaywrightEnvironment.ts

+      const getResult = <T>(
+        data: T[],
+        instances: BrowserType[] | Array<keyof typeof playwright.devices>,
+      ) => {


Suggested change

) => {

): any => {

mxschmitt · 2020-04-17T10:02:08Z

src/PlaywrightEnvironment.ts

+        data: T[],
+        instances: BrowserType[] | Array<keyof typeof playwright.devices>,
+      ) => {
+        const result: any = {}


Suggested change

const result: any = {}

const result: any = {} // eslint-disable-line no-explicit-any

mxschmitt

small nits. Would be cool to clean the ESLint warnings and use pragmas for the any cases. Then we can merge in my mind.

mmarkelov · 2020-04-19T13:39:42Z

small nits. Would be cool to clean the ESLint warnings and use pragmas for the any cases. Then we can merge in my mind.

Yeah! I agree but it seems challenging for me) To fix whole TS warnings

mxschmitt · 2020-04-19T14:03:20Z

small nits. Would be cool to clean the ESLint warnings and use pragmas for the any cases. Then we can merge in my mind.

Yeah! I agree but it seems challenging for me) To fix whole TS warnings

Feel free to use // eslint-disable-line :)

mmarkelov · 2020-05-07T08:18:24Z

I'll close it, cause I suppose using custom jest-runner is better solution for running tests for multiple browsers support
#113

mmarkelov added 4 commits March 19, 2020 12:56

New API

e032b14

Changes

1b7448d

Do not rubbish global with unnecessary variables

4cc1858

Adding browsers and contexts to global

b528f37

mmarkelov added 4 commits March 19, 2020 20:31

Replace array with object

d925ee2

Pass instances to proxy

ebb7f66

Fix instances

44c4eee

Add expectAllBrowsers helper

18e59b5

mmarkelov added 3 commits March 21, 2020 18:50

Improve some types

f409bbb

First step for support devices throw config

bf4f3c4

Update packages

1fb910b

mmarkelov added 12 commits March 22, 2020 17:38

Refactoring logic

c60e99d

Support config without devices

6701e8a

Some typescript improvements

d6ae7e5

Fix some others types

83d5fbf

TS common types

87500ad

Some changes

4847b5d

Merge branches 'New-API' and 'master' of https://github.com/mmarkelov…

ba2dee5

…/jest-playwright into New-API � Conflicts: � package-lock.json � package.json � src/PlaywrightEnvironment.ts

Update dependencies

b39ed95

Fix merging master

fc95666

Merge branch 'master' of https://github.com/mmarkelov/jest-playwright …

3d71a33

…into New-API � Conflicts: � package-lock.json � src/PlaywrightEnvironment.ts

Merge master

25696ee

Merge branches 'New-API' and 'master' of https://github.com/mmarkelov…

0cdef51

…/jest-playwright into New-API � Conflicts: � src/PlaywrightEnvironment.ts

Add prop to support new API

211f6ca

mmarkelov marked this pull request as ready for review April 9, 2020 16:43

mxschmitt reviewed Apr 9, 2020

View reviewed changes

mmarkelov added 3 commits April 10, 2020 10:15

Using import from playwright-core

1c6086b

Merge branches 'New-API' and 'master' of https://github.com/mmarkelov…

d56bb0d

…/jest-playwright into New-API

Rewrite selectors initialization

9d1274a

mmarkelov requested a review from mxschmitt April 12, 2020 09:13

mmarkelov added 3 commits April 15, 2020 15:33

Fixed error message

8fc3f52

Using new API to e2e test

96e40fe

Fix tests

74353da

mxschmitt marked this pull request as draft April 16, 2020 17:13

Fix tests

85f898c

mmarkelov marked this pull request as ready for review April 17, 2020 07:17

Merge branch 'master' of https://github.com/mmarkelov/jest-playwright …

c46e2b2

…into New-API � Conflicts: � src/PlaywrightEnvironment.ts

mmarkelov marked this pull request as draft April 17, 2020 07:19

Fix tests

bc8bc3b

mxschmitt reviewed Apr 17, 2020

View reviewed changes

mxschmitt approved these changes Apr 17, 2020

View reviewed changes

mxschmitt marked this pull request as ready for review April 17, 2020 10:04

Some ts fixes

67208bb

mmarkelov closed this May 7, 2020

mxschmitt deleted the New-API branch June 15, 2020 08:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New API #76

New API #76

mmarkelov commented Mar 19, 2020

mxschmitt commented Mar 19, 2020

mmarkelov commented Mar 19, 2020

mxschmitt commented Mar 19, 2020

mmarkelov commented Mar 20, 2020

mxschmitt commented Mar 21, 2020

mmarkelov commented Mar 22, 2020

github-actions bot commented Apr 9, 2020

mxschmitt Apr 9, 2020

mmarkelov commented Apr 16, 2020

mmarkelov commented Apr 17, 2020 •

edited

Loading

mxschmitt Apr 17, 2020

mmarkelov Apr 19, 2020

mxschmitt Apr 17, 2020

mxschmitt Apr 17, 2020

mxschmitt Apr 17, 2020

mxschmitt left a comment

mmarkelov commented Apr 19, 2020

mxschmitt commented Apr 19, 2020

mmarkelov commented May 7, 2020

	const result: any = {}
	const result: any = {} // eslint-disable-line no-explicit-any

New API #76

New API #76

Conversation

mmarkelov commented Mar 19, 2020

mxschmitt commented Mar 19, 2020

mmarkelov commented Mar 19, 2020

mxschmitt commented Mar 19, 2020

mmarkelov commented Mar 20, 2020

mxschmitt commented Mar 21, 2020

mmarkelov commented Mar 22, 2020

github-actions bot commented Apr 9, 2020

Pull Request Test Coverage Report for Build 47062a08620269e9ec26ff327fa6d35919c56745-PR-76

💛 - Coveralls

mxschmitt Apr 9, 2020

Choose a reason for hiding this comment

mmarkelov commented Apr 16, 2020

mmarkelov commented Apr 17, 2020 • edited Loading

mxschmitt Apr 17, 2020

Choose a reason for hiding this comment

mmarkelov Apr 19, 2020

Choose a reason for hiding this comment

mxschmitt Apr 17, 2020

Choose a reason for hiding this comment

mxschmitt Apr 17, 2020

Choose a reason for hiding this comment

mxschmitt Apr 17, 2020

Choose a reason for hiding this comment

mxschmitt left a comment

Choose a reason for hiding this comment

mmarkelov commented Apr 19, 2020

mxschmitt commented Apr 19, 2020

mmarkelov commented May 7, 2020

mmarkelov commented Apr 17, 2020 •

edited

Loading