add tests

[tleunen]

We should add tests to make sure there are no regressions when changing/adding new code...

[faergeek]

Which stack do you prefer? I'd like to try tape, do you have some experience with it?

[tleunen]

I don't really have much experiences in this.. Would probably have to check what other React repo do in term of tests and use the same frameworks for that...

[faergeek]

I'd like to help with that. Really want to have tests for this project.

[nathanmarks]

@tleunen If you're happy with karma+mocha for testing I can assist in writing tests -- just checking out the package now to potentially use in a project. If I do use it, I'll be more than happy to contribute tests.

[tleunen]

@nathanmarks sure thanks!
I'm still unsure if karma/mocha is really needed of if we just have to check the output html for each components based on the props we provide.
I don't think we really need to render it in a real browser in this case, because MDL upstream already does this kind of testing to making sure the rendering is correct.

[faergeek]

@tleunen It doesn't make sense to check rendered html and even check parts of a DOM, but we should test if interactions work correctly.

[faergeek]

Not sure if shallow render can help with that. MDL works in a browser environment.

[faergeek]

@nathanmarks Thanks, I think we need some help with tests.
Do you have any experience in writing tests with user interactions?

[nathanmarks]

@faergeek Are you talking about simulating real user interactions? I write quite a bit of scripting for headless browsers.

[faergeek]

Yes, react has test utils to simulate events https://facebook.github.io/react/docs/test-utils.html.

But I'm pretty sure that they're simulating events just for react itself and we probably need something like selenium to test the whole thing.

[nathanmarks]

@faergeek I have used selenium a bit and I've got a lot of phantomjs experience.

I think if @tleunen can pitch in and let us know what he sees as in scope for testing here -- we can make a decision on tape/mocha/etc.

I would start by asking ourselves, what is the goal of this package, and what is the best way to confirm that the goal is being reached?

If the goal is to 100% mirror the MDL upstream functionality, perhaps testing render output in different component states (via user interaction) and comparing it to the MDL output is a possibility for tests.

[faergeek]

@nathanmarks This project is not the mirror of upstream functionality, it's wrapper around it.

Probably we could try to just check the rendered DOM in different states, but I'm not 100% sure that it will be enough.

About testing framework: I suggest using tape as a testing framework because TAP is widely supported standard and we potentially get many integrations for coverage, CI and such things related to testing.

But I have no experience with it.
I'm going to try it out on a smaller project like lib which I didn't publish yet or on one of my private react projects to see how smoothly it goes.

[tleunen]

I don't think Selenium is needed.
What we just have to make sure is the output of the components based on the props they receive.

Mocha, chai or whatever should be enough.

[faergeek]

Yes, probably we don't need selenium or even phantom for now.

About "mocha, chai or whatever":

Mocha is a test runner and can't work by itself, it has no assertion library, chai is assertion library that works with mocha.

Tape on the other hand is a standalone tool that runs tests and also provides assertion library, but it needs a reporter probably (standard reporter just outputs raw test data to console, which is not so readable).

[tleunen]

I'll probably go with Mocha + basic assert (at least for now)

[faergeek]

Ok, I just told about advantages of tape.

If you still wanna to use mocha, let's use it 👍

[nathanmarks]

I'm happy to contribute tests with either of them (tape or mocha).

Tape is super simple and easy to use.

[joshq00]

If you'd like to use karma and tape, I've started a branch on my fork.

If not, I'll wait until you get your testing approach sorted out before writing more tests

[tleunen]

I've started to use mocha + React shallow renderer in this PR #84

Again, I'm not sure something complexe with browser testing is required in this case because MDL already does this part to be sure everything is rendered properly across browser.

[faergeek]

@tleunen I think we should at least use strictEqual method of assert because assert.equal uses == instead of === which is not cool in many cases, especially for tests.

Or just take a look at what @joshq00 did with tape 👍
I still think that it's more readable and just simpler.
We don't need karma though.

But we definitely need code coverage + some utils for simpler shallow rendering + watching for development.
I'm going to make a branch with all of it right now :-)

P.S. I'm also going to look at that cool thing http://speckjs.github.io/

[faergeek]

@tleunen Take a look at this article about tape https://medium.com/javascript-scene/why-i-use-tape-instead-of-mocha-so-should-you-6aa105d8eaf4

[tleunen]

About assert, that's why I started using expect instead. It gives more utils and better equals.
About tape vs mocha. Some people prefer the former, some other the latter. I chose the latter for no particular reason except it's widely used. I know tape is also well known.

[faergeek]

Tape is well known, small and predictable, which is important for tests.

I think react and co has popularized unix-way (small tools that each do one task, do it well and can be integrated vs do-anything combines) in frontend and it's good ;-)

I don't know the reason why mocha has no assertion library, but have many rarely needed things, like tons of reporters etc. It's bad separation IMHO.

And just look at tape, it's much more readable anyway.

[joshq00]

I would actually argue that less assertion functions is better.

When I used mocha and chai, learning the million different asserts was time consuming. Writing tests, I was constantly referencing the docs

Sticking to just a few forces you to simplify what you're testing. I try not to use deep equals or any of those. As much as I can, testing with .equal keeps them very clear about what I'm expecting and makes me better design my tests

[faergeek]

@tleunen What if we at least agree on using only simple assertions as @joshq00 said?

[tleunen]

What's wrong with expect? It's easy to use.

[faergeek]

Nothing's wrong.
Let's just agree to not use some assertions like expect(3).to.be.not.ok and instead use expect(!3).to.equal(true), for example.

[tleunen]

Not sure to see the point here. I'm using what feels the best to use for every usecase.

[faergeek]

I mean it takes time to search docs for method that "fits" and then search what exactly method does when reading.

I did it before, it annoys. Looks logical only at the first time.

[joshq00]

My question is what is gained when using the long-chained language asserts?

expect( obj ).to.have.property( '_id' ).that.is.a( 'string' )

does not give any added value over

assert( typeof obj._id === 'string' )

But it does make it take longer to write the tests.
"Do I use .that.is.a.number? or is it .that.is.a( 'number' )? I forget... check the docs"

[faergeek]

@joshq00 Exactly 👍

[tleunen]

It's not that long. Take a look https://github.com/mjackson/expect
And it gives you readability.

[faergeek]

Okay, that version is much better than embedded in chai 👍

[tleunen]

I made a few tests with jsdom this morning.. Well, it's a lot faster than Karma, but a few things are not implemented in jsdom (like html5 validity inputs) which prevent some tests to be done :/