Firefox - Cookie unit tests fail for asserts on non-standard properties (eg sameParty)

[juliandescottes]

Puppeteer recently started to assert new cookie properties in unit tests:

• sourcePort & sourceScheme in feat(chromium): roll Chromium to r856583 #6927
• sameParty in feat(chromium): roll Chromium to r843427 #6797

They seem non-standard. At least for sameParty I could find a reference clearly stating that Firefox would not support it: https://www.chromestatus.com/feature/5280634094223360
For sourcePort & sourceScheme I didn't find any resource.

Firefox currently has no way of supporting such properties, so all the tests asserting those values will fail on Firefox when we update Puppeteer:

• puppeteer/test/cookies.spec.ts

  Line 34 in 35e66c4

  itFailsFirefox('should get a cookie', async () => {

• puppeteer/test/cookies.spec.ts

  Line 90 in 35e66c4

  itFailsFirefox('should get multiple cookies', async () => {

• puppeteer/test/defaultbrowsercontext.spec.ts

  Line 27 in 35e66c4

  itFailsFirefox('page.cookies() should work', async () => {

  
  (only mentioning the tests which used to pass before asserting the new properties)

I realize that in puppeteer those tests were already running as itFailsFirefox before, so the regression was not detected. But they were actually passing and we are running them successfully in the Firefox CI today

Firefox will probably never be able to pass the assert for those properties, but it would be great to be able to still run the tests, maybe we could check the non-standard properties only when running in Chrome?

(corresponding Bugzilla reference: https://bugzilla.mozilla.org/show_bug.cgi?id=1694506)

[whimboo]

@jschfflr and @mathiasbynens could we get those extra checks put behind an if condition for Chrome?

[juliandescottes]

FWIW, I tried a simple modification to skip sourcePort, sourceScheme and sameParty from the asserts on cookies when isChrome is false and this fixes the tests for Firefox, so it's really just about those three properties (changeset, ci job).

Mostly extracted the logic to a small helper to assert cookies.

export const expectCookieEquals = (cookies, expectedCookies) => {
  const { isChrome } = getTestState();
  if (!isChrome) {
    for (const expectedCookie of expectedCookies) {
      // Remove non-standard properties when testing another browser than Chrome.
      delete expectedCookie.sameParty;
      delete expectedCookie.sourcePort;
      delete expectedCookie.sourceScheme;
    }
  }

  expect(cookies).toEqual(expectedCookies);
};

[jschfflr]

Sounds good to me! Feel free to upload your change as a pull request here and I'm happy to accept it!

[juliandescottes]

Thanks! opened #6994

[juliandescottes]

This can be closed as #6994 was merged