electron-userland / spectron

DEPRECATED: 🔎 Test Electron apps using ChromeDriver
http://electronjs.org/spectron
MIT License
1.68k stars 229 forks source link

Rewrite Discussion #1044

Closed goosewobbler closed 2 years ago

goosewobbler commented 2 years ago

I thought I'd put an issue up to discuss my rewrite of Spectron, seeing as forks aren't allowed their own issues, etc.

Is a rewrite necessary given that Playwright seems to be working well? Possibly not, but I started so I'll finish. Can't hurt to have another option, and this approach gives us all the benefits that come with close WDIO integration.

I think the WDIO-first approach works well, though would like to get back to a place where Chromedriver restarts between tests as I don't think the test state bleed within spec files is expected or desirable. The idea with this is to create a wdio-spectron-service that is a worker service and can better manage the CD process.

It's still very much a work in progress but I spent a lot of time documenting things, constructive comments welcome.

https://github.com/goosewobbler/spectron

goosewobbler commented 2 years ago

Progress update, I got the service afterTest hook working after following @christian-bromann's suggestion to run the WDIO testrunner directly through the CLI.

I'm trying to recreate this behaviour of the original Spectron:

https://github.com/electron-userland/spectron/blob/master/lib/application.js#L103

Current afterTest looks like this:

    await this.browser?.exitElectronApp();
    await delay(1000);
    await this.killProcess();
    await this.startProcess();
    await remote(this.wdOpts);

Now have CD restarting and an Electron instance for each test but the windows are not being closed after each test, and I think WDIO is not able to find the new instances as it's coming up with a load of no such window: target window already closed errors. All work in the chromedriver-service branch if anyone has any ideas:

https://github.com/goosewobbler/spectron/pull/10

Running tests through pnpm test:local when in the test directory.

goosewobbler commented 2 years ago

Another progress update, After chatting with @christian-bromann we decided to create a new service in the webdriver-community repo for this. I will move things over there in time (will take a while to extract my original custom service from my fork monorepo). Now using wdio-chromedriver-service in tandem with the new service and not restarting chromedriver. The new service just handles the capability munging and exits the app / spins up a new session between tests.

Still getting the same error as before, afterTest now doing something like this:

    await (browser as SpectronClient)?.exitElectronApp();
    await delay(1000);
    await remote(this.wdOpts);

All work now in the wdio-electron-service branch:

https://github.com/goosewobbler/spectron/pull/13

h0p3zZ commented 2 years ago

I don't really know where to put this so I'm just asking here (just tell me where to put it and i'll move it there)

I'm trying to setup your spectron fork for our electron project. We are using Vuejs and I'm not really sure how to set it up for js any tips?

goosewobbler commented 2 years ago

@h0p3zZ I honestly would wait until it's a bit further along, you're basically trying out alpha version code right now. I haven't had much time to work on this recently as I'm just getting my own electron project to the first release stage.

If you still want to try and don't mind having to work around test state bleed within spec files (you will likely have to change your tests once I have got electron restarting for each test), then I think the docs are relatively good for the current build? You just import the main and preload scripts, add a config file and write some tests which look like the example.

h0p3zZ commented 2 years ago

@goosewobbler Yea but i'm not quite sure where to import the maina nd preload scripts? And also i tried setting it up for some time now and i actually was pretty close to get it to work but i somehow got the same js error over and over again and i had no idea what to change so i just went back to the normal spectron we where using before.

By the way is there any way to just get the spectron instanze and not the spectronApp? would be great for vue.

goosewobbler commented 2 years ago

So there is no spectron instance any more, it's a complete rewrite and WebdriverIO handles the chromedriver process now. I don't see how the old approach would help with respect to using Vue. SpectronApp is just a bridge providing access to Electron APIs of your app and some of the webdriver browser helper functions that the old spectron used to use. You import the preload script at the top of your preload.js, if you're not using one, you need to create it. The main script goes at the top of your main process index.js - that's where your app entry point, where you create the main window.

You haven't given me any details of the error but it sounds like you are best off sticking to the old Spectron or migrating to Playwright.

h0p3zZ commented 2 years ago

Thanks for the help - I'll maybe retry it in the future because the approach you are taking looks very good!

The thing for vue is that there is a plugin for building vue electron apps. This node module also provides a function for building and then running spectron tests so thats why i thought of the spectron instance.

I'll come back to you if i find anything new that might help you with your project.

goosewobbler commented 2 years ago

Yeah, that vue plugin will need to be updated for the new approach if they're integrating with Spectron. However, the code I have written is more likely (after a lot more work) to end up in a WebdriverIO repo, hopefully becoming the standard approach to testing Electron apps with WDIO.

h0p3zZ commented 2 years ago

Would be great, looking forward to it.. keep up the great work :)

goosewobbler commented 2 years ago

Another update here, I have released https://github.com/webdriverio-community/wdio-electron-service.

My fork of Spectron is so far removed from the original, it will work better as an extension of WDIO. The new service effectively replaces the run.ts functionality from the fork, restarts the session between tests and also wraps wdio-chromedriver-service. Next, I will be looking to integrate the remaining code from the fork so that electron API access is available on the WDIO browser object.

goosewobbler commented 2 years ago

After a bit of investigation I'm not sure much of the Electron API access is needed. WDIO and its ecosystem provides a lot of options for testing and while there's definitely scope to add some electron-specific stuff and helpers to the browser object, I don't want to head down the route of adding functionality for the sake of it; I feel like the original Spectron had a fair amount of unused/pointless functionality which didn't actually benefit real-world testing.

I'm going to be using wdio-electron-service to test my own Electron project so no doubt I'll find some rough edges to smooth over with more code. Currently trying to work out why @testing-library/webdriverio times out looking for a button.

goosewobbler commented 2 years ago

I'll close this issue now, added a deprecation notice to my Spectron fork, would appreciate if people could try out the new service and file bugs / feature requests where necessary. Cheers.