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

Add unit tests #27

Open
matheuss opened this issue Oct 6, 2016 · 30 comments
Open

Add unit tests #27

matheuss opened this issue Oct 6, 2016 · 30 comments

Comments

@matheuss
Copy link
Member

matheuss commented Oct 6, 2016

We need some tests:

@zeke
Copy link

zeke commented Oct 6, 2016

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.

@matheuss
Copy link
Member Author

matheuss commented Oct 6, 2016

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

@sindresorhus
Copy link
Contributor

@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
Copy link

zeke commented Oct 7, 2016

@sindresorhus sweet! I will give AVA another go.

@hyalkaf
Copy link

hyalkaf commented Oct 8, 2016

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

@matheuss
Copy link
Member Author

matheuss commented Oct 8, 2016

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

@matheuss matheuss added this to the 1.0.0 milestone Oct 8, 2016
@matheuss matheuss removed this from the 1.0.0 milestone Dec 7, 2016
@fokusferit
Copy link
Contributor

Any progress here ? 😄

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

@sindresorhus
Copy link
Contributor

@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
Copy link

brodeynewman commented Sep 3, 2018

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
Copy link
Member

skllcrn commented Sep 3, 2018

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

@brodeynewman
Copy link

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.

@skllcrn
Copy link
Member

skllcrn commented Sep 3, 2018

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

@sindresorhus
Copy link
Contributor

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

@brodeynewman
Copy link

Expect a PR for some tests soon 😄

@bringking
Copy link
Contributor

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

@dhuang612
Copy link

dhuang612 commented Sep 11, 2019

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

@dhuang612
Copy link

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
Copy link

never mind I understand my issue

@dhuang612
Copy link

dhuang612 commented Sep 12, 2019

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

Screen Shot 2019-09-12 at 4 23 44 PM

@dhuang612
Copy link

dhuang612 commented Sep 13, 2019

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
Copy link

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
Copy link

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
Copy link

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
Copy link

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
Copy link

dhuang612 commented Sep 17, 2019

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
Copy link

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

@skllcrn
Copy link
Member

skllcrn commented Sep 23, 2019

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!

@skllcrn skllcrn changed the title We need tests Add unit tests Sep 23, 2019
@dhuang612
Copy link

Hi,

created PR #730 for this issue.

@dhuang612
Copy link

added some more tests

@makhambettorezhan
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants