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

test: rework E2Es #956

Open
wants to merge 48 commits into
base: main
Choose a base branch
from
Open

test: rework E2Es #956

wants to merge 48 commits into from

Conversation

goosewobbler
Copy link
Member

@goosewobbler goosewobbler commented Feb 14, 2025

Reworking the E2Es so that they use a matrix-based approach and the packaged version of the service.

Closes #955

TODO:

@goosewobbler goosewobbler added the status:wip Work in progress label Feb 14, 2025
@goosewobbler goosewobbler self-assigned this Feb 14, 2025
@goosewobbler goosewobbler added the backport:requested Should be applied to the maintained (previous) version label Feb 14, 2025
@goosewobbler
Copy link
Member Author

Targetting 8.0.2 for this.

@goosewobbler goosewobbler removed the backport:requested Should be applied to the maintained (previous) version label Mar 4, 2025
@goosewobbler goosewobbler added this to the 8.x.y milestone Mar 4, 2025
@goosewobbler goosewobbler added release:future Include in a future release (not the next one) semver:patch Changes that don't affect the API (bug fixes) track:main For the current stable version track labels Mar 5, 2025
@goosewobbler goosewobbler force-pushed the sm/packaged-e2es branch 2 times, most recently from 07754e5 to 1d0bade Compare March 7, 2025 15:29
@goosewobbler goosewobbler changed the title Update E2Es to use packaged service Rework E2Es Mar 7, 2025
@goosewobbler goosewobbler force-pushed the sm/packaged-e2es branch 2 times, most recently from 8f7428b to 64c0acc Compare March 11, 2025 12:09
@goosewobbler goosewobbler linked an issue Mar 11, 2025 that may be closed by this pull request
@goosewobbler
Copy link
Member Author

goosewobbler commented Mar 11, 2025

@mato533 Just to let you know I thought I'd do the yarn hack removal in this PR, and rework the E2Es at the same time. Basically deal with all the tech debt before getting some traction on v9. Let me know what you think of this matrix approach to the E2Es, and if you have any improvements.

I decided to ditch the JS variants of the E2Es for now in order to make the rework simpler, these variants were put in place for a specific bug that was masked by the use of tsx so I'm not sure how much value we are getting from them vs. maintenance cost and build slowdown. We can always reinstate them after the rework.

@mato533
Copy link
Member

mato533 commented Mar 12, 2025

@goosewobbler
It is awesome idea to rework E2E before v9.

Basically, The matrix approach seems to be good idea.
This service should be tested to the forge and builder. (I agree that you have decided to remove the JS variants considering into historical background)

I've put together some points that I think need improvement. I hope it helps.

  • improvements (May be)
    • Number of the Sample App
      Which test target electron app is correct?

      1. An electron app which build result is based on CJS/ESM code
      2. An electron app whose source code (which before build) is written in CJS/ESM and bundled with it
        If this is correct target, we have to check the current sourcecode... Although it's named cjs, it looks like there are sample apps written in ESM.

      If the a) is correct, we may be able to reduce the number of apps from 6 sample apps to 3 because we can build both CJS and ESM from ESM code by creating properly configurations for the Rollup.(When remove the JS variants, it become 4 sample apps to 2 apps)

    • E2E concurrency
      E2E needs to run 16 patterns of E2E, 4 (Lin/Mac/Mac-uni/win) multiply 2 (builder/forge) multiply 2 (esm/cjs).
      there is 2 approach in order to execute these efficiently,.

      1. Execute parallel in multiple container while taking into account the limitations of Github Actions (The sample of this approach is ci: improve performance of CI job #978)
      2. Execute parallel in a single container, taking into account the CPU/Memory consumption of the container
        This approach is similar to current E2E for Mac and Linux.
        But current E2E is executing sequentially the scenarios (no-binary-esm -> no-binaly-cjs -> builder-esm ->....)
        This approach is intend to running multiple scenarios at the same time.
        build no-binary-esm -> run no-binary-esm
        build no-binary-cjs -> run no-binary-cjs
        build builder-esm -> run builder-esm
        ....

      In practice, The approach a) seems to be the good approach, as there are variations in performance between operating systems and build tool combinations (Windows is generally slower, Mac-Universal is slower to build, etc.). This is only my opinion, I respect your thought.

@mato533
Copy link
Member

mato533 commented Mar 12, 2025

Incidentally, it is assumed that the dependency between sample-apps and the service will completely disappear from v9. (because wdio-electron-service will have no more preload/main scripts to import due to the IPC-Bridge replacement).

This should allow e2e app builds to run in parallel with serivce builds, which may be able to improve things in some way when v9 is released.

@goosewobbler goosewobbler force-pushed the sm/packaged-e2es branch 3 times, most recently from 8cb54e8 to d754d48 Compare March 21, 2025 15:51
@goosewobbler goosewobbler force-pushed the sm/packaged-e2es branch 5 times, most recently from 4b23b5c to 2fe5a77 Compare March 21, 2025 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:next Include in the next release within its track semver:patch Changes that don't affect the API (bug fixes) status:wip Work in progress track:main For the current stable version track
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test E2Es against packaged service instead of workspace link
2 participants