Add unit tests

[wulkano]

Add unit tests

[zeke]

https://github.com/electron/electron-api-demos/blob/master/tests/index.js is a pretty good Spectron example to start from.

I tried AVA for a bit but it was quite slow to start up every time because it's doing so much transpilation.

[sindresorhus]

@zeke That should only be the first run. We cache everything. Unless you're using babel-register, which circumvents our cache, but I don't see why babel-register would be used here. Node.js 6 also improved require time significantly, so should help. We're working on a promising solution: avajs/ava#1052.

[zeke]

@sindresorhus sweet! I will give AVA another go.

[dhuang612]

Hi guys,

I know a number of people have already tried to take on this PR.
I am a complete beginner when it comes to writing tests.

I've just added in a sample test to run and confirm that it is setup correctly.
I will probably be back in a bit to ask some questions.
Currently using the two packages Spectron and Ava as suggested from the original comment

Thanks

[makhambettorezhan]

Been using your app and it is very simple. It would mean great deal to me to contribute to this project. I've seen some of the people already added some tests but I'd like to add some more. And for now I'm learning examples of AVA and Spectron. Hope I can add some tests soon.

[matheuss]

Thanks for the link @zeke – will be needed since I never used Spectron 😅

Yeah, AVA takes some time to start – but it's awesome 😌 cc @sindresorhus

[hyalkaf]

I would like to start working on this. Maybe I can start doing tests in chunks and make pull requests for them?

[brodeynewman]

Awesome. I will start on this today then.
Also, do we have a preference on what test libraries / frameworks we use? I usually go with Jest due to Istanbul coverage reporting being built in, and mix Enzyme in for React component testing.

[brodeynewman]

Expect a PR for some tests soon 😄

[skllcrn]

I highly suggest creating a markdown table with what you intended to cover with the tests, that way we have a better idea of what exactly we aim to cover @dhuang612 🙏 Thank you for bringing this issue back to life!

[matheuss]

Sounds amazing @hyalkaf 😄 You could write the first ones and then send a [WIP] PR and then just keep adding the others 😄

[fokusferit]

Any progress here ? 😄

Otherwise I would try to start on it. Is the decision on AVA now final or Spectron ?

[sindresorhus]

@fokusferit No progress. Feel free to take it on. AVA + Spectron. Example here: https://github.com/electron/spectron/blob/master/README.md#with-ava

[brodeynewman]

Has any progress been made on this yet? I'd like to start chunking away at this.
First time contributing to this project btw.

[skllcrn]

That'd be fantastic @brodeynewman! There hasn't been any substantial progress on this, you can grab the latest version of Kap off master, here's how to get set up: https://github.com/wulkano/kap/blob/master/contributing.md

[skllcrn]

That makes sense, you can also look into AVA with Enzyme!

[sindresorhus]

I would prefer AVA and Enzyme. See: https://github.com/avajs/ava/blob/master/docs/recipes/react.md Make sure you install yarn install [email protected] (latest version). For code coverage, see: https://github.com/avajs/ava/blob/master/docs/recipes/code-coverage.md

[bringking]

@brodeynewman any progress on this? would love to be able to test changes in here

[dhuang612]

okay,

So I got it setup in package.json and hooked it up to npm test.
When I setup the test.js page following this guide:
https://github.com/electron-userland/spectron#with-ava

I get the following error:

 TypeError: Tests must have a title

So I opened up a new issue on spectron's repo
electron-userland/spectron#434

[dhuang612]

never mind I understand my issue

[dhuang612]

sent a PR could I get a review on work done so far?

[dhuang612]

just wanted to send a quick update.
I have made the changes that the reviewer requested.
Right now circle ci is timing out on this ava test without ever running it.
The test passes both on my local machine, and when I push the commit to this repo.
Investigating into circle ci.

Included screenshots into my PR

[dhuang612]

opened up an issue with this repo

electron-userland/spectron#439
https://github.com/electron-userland/spectron

still getting the same issue of having the test timeout when using ci

[dhuang612]

been reviewing documents on spectron and ava to try and find the correct param I am missing to get this working on circleci. Since the no output for 10 minutes error means that the environment isn't being setup correctly.
So it seems like I need to start setting up some sort of way to fake the browser existing for the ci tests to pass?
I've been looking into this
https://blog.logrocket.com/introduction-to-headless-browser-testing-44b82310b27c/
and will need changes to be made to the config.yml

[dhuang612]

I am basing my previous statement from this article available on electronjs which talks about headless testing and mentions circleci

https://electronjs.org/docs/tutorial/testing-on-headless-ci

currently reading through circleci docs

[dhuang612]

Hello,

I have a question, could someone tell me if the circleci logs are saying anything?
The only response I am getting from that for my tests is a timeout which isn't a helpful message.

I understand that spectron has webdriverio as a wrapper so that should be enough to run it?

[dhuang612]

Hey guys,

So a docker image needs to be setup to replicate the environment so that these tests can be run.
Here is an article about it from circleci
https://circleci.com/blog/docker-what-you-should-know/
https://circleci.com/docs/2.0/building-docker-images/

Basically tests can't be added until this is configured, that's why the timeouts have been happening.
Could you guys setup an electron docker and then I can get this test to pass and continue to work on others?

Here's an example of another github repo with working tests and their config.yml has a docker image setup for electron

https://github.com/CityOfZion/neon-wallet/blob/dev/.circleci/config.yml

Thanks guys

[dhuang612]

I now have a better understanding of next steps. I'll get working on this and resubmit the pr

[dhuang612]

Hi,

created PR #730 for this issue.

[dhuang612]

added some more tests