search

GoogleChromeLabs / chromium-bidi

Implementation of WebDriver BiDi for Chromium
https://googlechromelabs.github.io/chromium-bidi/
Apache License 2.0
87 stars 30 forks source link

Add e2e / wpt test coverage for Windows and Mac and track improvements to CI in general #361

Closed OrKoN closed 1 year ago

OrKoN commented 1 year ago

Currently, the implementation is only tested against Linux binaries. We should expand the testing to Mac and Windows, and possibly headful, headless, new headless to catch flakiness and platform differences early.

Tracking

sadym-chromium commented 1 year ago

I'm not sure if we should run both new headless and headful. And not sure if we should run all 3 options in all 3 OSes. We can start with running only "new headless" in Mac and Win.

UPD: We have 4 dimensions:

  1. OS (Linux, Mac, Windows);
  2. Headless (headless old, headless new, headful);
  3. Tests (e2e, WPT);
  4. Runner (ChromeDriver, standalone mapper).

Which ends up in 36 different options. IDK where we should put the bar of coverage to be "good enough", but currently we don't have any coverage of windows and of the old headless.

thiagowfx commented 1 year ago

@OrKoN: @sadym-chromium suggests we do not need macOS coverage at this time (e.g. as delivered by https://github.com/GoogleChromeLabs/chromium-bidi/pull/427), as we do not iterate that quickly.

Should we close this issue?

mathiasbynens commented 1 year ago

What’s the motivation behind not wanting to keep the macOS coverage?

macOS support is crucial for BiDi-in-Puppeteer. If we don’t test it at this repository’s level, how can we be confident that the integration into Puppeteer works?

sadym-chromium commented 1 year ago

My suggestion is not to abandon but to deprioritise it. The issue with obviously higher priority is #357

thiagowfx commented 1 year ago

https://github.com/GoogleChromeLabs/chromium-bidi/pull/427 is already written so you can consider it a low-hanging fruit to enable E2E on macOS.

That said, yes I agree this is a good time to shift gears and switch into some real feature work / fixing tests, as our CI got in really good shape recently.

thiagowfx commented 1 year ago

Added a Status section to the first comment. I don't think we want that explosion of CI combinations though.

Could you please add notes to the first comment to clarify which combinations we're NOT interested in?

thiagowfx commented 1 year ago

I'll close this as a duplicate of https://github.com/GoogleChromeLabs/chromium-bidi/issues/876. It's trivial to make combinations of {linux, mac} x {headful, headless} x {wpt, e2e}.

The difficult part is to add Windows support, which is more appropriately tracked in the other bug.