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

Better tests and code organization #415

Closed
tunnckoCore opened this issue Jan 16, 2017 · 10 comments · Fixed by #545
Closed

Better tests and code organization #415

tunnckoCore opened this issue Jan 16, 2017 · 10 comments · Fixed by #545

Comments

@tunnckoCore
Copy link
Member

tunnckoCore commented Jan 16, 2017

I mean, we already have few issues because the way the whole code is written - I'm talking for the GENTLY thing at first line of each file ( #337 ). Tests are very confusing and not so easy to maintain. Switching to something more meaningful for whole community would be great - both for us, newcomers and new contributors.

And I'm not talking to switch to something fancy and shitty like AVA or whatever. But the whole testing organization should be changed and rewritten:

  1. mocha - nah.. too big code, shit and in bonus: globals; no thanks (self pref. i stopped using it, used it for about few months when i enter in nodejs - I don't see fit using it soon after that. It is not just a selfpref. it is too big with no reason)
  2. ava - mega big, mega shit, too fancy. selfpref: just seeing a very little bit sense some to use it only if project is huge/enterprise
  3. testit - small, compact, using promises and support nested groups and tests (needless); selfpref: used it through my thin wrapper assertit for 1 year - seems good in most cases, but has bad debugging experience
  4. mukla - absolutely thin with support for streams, callbacks, promises, child processes, async/await and sync tests (ran in parallel), no support for nested groups and tests (enforce atomic tests, if test suite is huge separate it to files), meaningful testing mocha/tape/testit-style using it from 3 years. It is absolutely stable and debugging is cool (has some edge cases that sucks, but it is because the architecture) - at least has clean stacktraces - 1-2 meaningful lines max. But i'm releasing v1 soon (even it is written i'm just blocked because other packages and switching workflow tools). Compared to the other similar testing libs is very good and almost 1:1 (porting from mocha-style + assert to mukla is for ~1min) without the cost for huge install times and huge deps.

I have pretty good experience about testing and building core blocks of such tools. I have written a few testing libs few times. And I always try to stay to standards and conventions, so to not have learning curve. In the case with mukla it is just enough to switch it with test or just make var it = require('mukla'). I just have few ideas for different API approaches and this can be seen in the v1 issue.

mocha

var assert = require('assert')

it('foo bar', function (done) {
  assert.strictEqual(1, 2)
  done()
})

mukla

var test = require('mukla')
var it = test // main export is function
var assert = test // mukla exposes all `assert` methods

it('foo bar', function (done) {
  assert.strictEqual(1, 2)
  done()
})

With easy migration path mukla v1 (mocha-style) -> gruu v1 (compat layer/mix of tape-style and mukla/mocha-style) -> asia v1 (ava/tape-like).

But anyway. I may gone too offtopic, but hope it make sense.

So in short, if you want forget all I said previously, but we need to change two things - 1) rewrite tests, using some meaningful lib and something like superagent; 2) fix the GENTLY thing

/cc @badeball @pornel (sorry for the typos if any, kinda dyslexia ;d)

@kornelski
Copy link
Contributor

kornelski commented Jan 16, 2017

GENTLY tests are indeed problematic and I'm in favor of changing to some other solution that doesn't require modification of the code in test.

Calling mocha and AVA shit is harsh. I don't share that sentiment.

I'm not sure what's the problem with "big". I'm not concerned about code size of the testing framework, since the framework is not a runtime dependency. If you mean "big" in terms of features/API that would create a big learning curve, then at least in case of mocha I don't feel that — you can get away with knowing just 2-3 functions.

I don't mind the few globals that mocha uses — they're not mutable, and since formidable isn't going to export any globals, they won't cause any conflicts either.

The only thing I find truly problematic about mocha is that it's single-threaded and doesn't support parallelizing tests (there's a mocha.parallel library, but it has quirks and limits). I think ability to parallelize tests is quite important, especially if the code under test may need to touch the network or test timeouts.

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 16, 2017

Don't want to continue that path, intentionally, as I said "self prefs". I'll just clarify a bit:

since the framework is not a runtime dependency.

They aren't but it takes unnecessary of the dev time, too big install times.

If you mean "big" in terms of features/API that would create a big learning curve

No. I not meant that. Mocha is here for years and almost everything is sit on top of these conventions and that style. AVA sits on top of tap(e), so I believe it is not a problem for anyone to switch between them without problems. There almost no learning curve between testing frameworks, exactly because of this. The difference comes for install times (believe me it matters) and how fast they are in running the suites.

Install times even of the devDeps are very important, and should be important not only for me. I hate to have 10-20+ huge devDependencies that has up to 20-40 deps each - some of them has lower than that but are very deep, some of them has more but they are flat and etc and etc. It matters, believe me.

They are awesome in some viewpoint. They are here for years and they will stay for years. But it's very important that we should balance the things. It just not make sense to has some huge devDependency instead of some X times faster and X times smaller with same API and features. And if someone is author of such small package almost compatible with the biggest names, it not means that it not respect The Ones. And if that unknown package has not much stars and popularity not means it is not awesome as much as the most popular and used.

I never talk just for that to have some chat. I'm here for years, know their code bases line by line and know that the things could be done times better, so I'm doing it. They are cool and fancy, they are easy to use, they are ready for up and running, they are great for these users that just want the things to work and they don't know the internals (and they don't care) of the code that they use every day. And yea I would use AVA in some huge project or app - it shines there, but it is absolutely overkill for "small modules philosophy" modules.

But yea, again, self preferences. 🍻 No bad feelings, sorry if I sounds aggressive or something... That's just my style and sometimes I don't find correct words and sentences, because English is not my mother language. Just love to share my thoughts, experience and feelings :)

I think ability to parallelize tests is quite important

Absolutely agree.

@kornelski
Copy link
Contributor

I agree that installation of packages with lots of deps via npm is annoyingly slow.

However, I think it'd be a shame to reject otherwise good libraries because npm is slow to install them. There could be alternative solutions:

  • We can recommend using npm i --production or Yarn
  • If we pick a popular library, then there will be a high chance that users already have it installed (because another dependency requires it), and in such case we won't be making things slower.
  • In worst case can make a copy of the library with dependencies bundled in (e.g. webpack all the deps) and depend on that. For npm that will be one tarball with one file to install, so hopefully it won't be that slow.

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Jan 16, 2017

I tried everything and started using Yarn from day 1, but it is still slow for me (i'm with great connection - Bulgaria is in top places by best and fastest internet for years). But yea, maybe I'm too paranoid, haha.

Anyway. Nevermind. It is total offtopic ;d
We are both agree that marked things in first post should be changed :)

Cheers.

@GrosSacASac
Copy link
Contributor

I'll have a look at the tests at some point. We could also use jasmine.

@GrosSacASac GrosSacASac pinned this issue Nov 28, 2019
@GrosSacASac
Copy link
Contributor

@tunnckoCore
Copy link
Member Author

jest and superagent with koa and express

@GrosSacASac
Copy link
Contributor

GrosSacASac commented Dec 20, 2019

ava is broken in Node13 avajs/ava#2293

@tunnckoCore
Copy link
Member Author

tunnckoCore commented Dec 20, 2019

I'm against AVA, million deps for nothing (and that's not the only thing), evenmore we are targeting node 10.

btw, we should only test and support LTSs

@tunnckoCore
Copy link
Member Author

Most of the things are done. Need only to switch the test runner (to Jest + superagent/supertest) which is kind of easy thing too.

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

Successfully merging a pull request may close this issue.

4 participants