Skip to content
This repository has been archived by the owner on May 13, 2019. It is now read-only.

Factor out all Node.js core modules interface tests to become their own modules #50

Closed
daviddias opened this issue Jan 29, 2017 · 6 comments

Comments

@daviddias
Copy link

Lacking to know where's best to open a issue about this matter, this one looked the one most suited.

I would like to propose the extraction (or perhaps creation in some cases) of the tests that validate the interface of each Node.js core module. Through this refactor, we would have batteries of tests that can be used for the several Browser shims that exist today, avoiding duplication of tests and cases where something is left out of the interface.

It has been the case, where a shim was not 100% to the expectations of the shimmed code creating errors in runtime or the development got blocked by not having an easy+quick way to test the shim against the expectations of Node.js core (i.e).

This thought has been in the back of my mind for a while and now that I took over node-spdy(which also implements http2), I felt it would be a huge opportunity to leverage all the work @indutny put into the development of this module and test it hand to hand with the development of http2 in Node.js core. Note that node-spdy is not a browser shim, instead, it is a full implementation of SPDY3, SPDY3.1, and HTTP2 in JavaScript.

This follows the pattern used by things like abstract-blob-store, interface-stream-muxer and others.

A next interesting step, if this one gets traction, would be to provide more than interface tests, so that in the same way that we have Buffer performance tests across envs, we can test all the shims with the same benchmarks and see how they behave in different browsers.

@refack
Copy link

refack commented May 23, 2017

Interesting... 🤔
Since [meta: decharter the Docs and Testing Working Groups] let's move this to core. Create a tracking issue with breakdown of the needed tasks a la nodejs/node#11233.
I assume you'd want to start with http & https and the http2 from https://github.com/nodejs/http2/tree/master/test/parallel?

@Trott
Copy link
Member

Trott commented May 24, 2017

I'm not necessarily opposed to this, but I'm not sure there's a solid reason it should really be the job of core. There are already modules out there that replicate/track core modules (or at least used to) like https://github.com/jscissr/http-node. Someone could do the same for the tests.

@refack
Copy link

refack commented May 24, 2017

Not trying to be grandiose, but it'll give our API "standard" like properties, a la https://github.com/w3c/web-platform-tests. Something to consider...

@daviddias
Copy link
Author

Exactly, https://github.com/w3c/web-platform-tests is a great example of this.

Create a tracking issue with breakdown of the needed tasks a la nodejs/node#11233.

@refack should this issue be a simple list of each core API and say "factor tests out"? Looks like those issues are for things already approved, no?

@refack
Copy link

refack commented May 29, 2017

@refack should this issue be a simple list of each core API and say "factor tests out"? Looks like those issues are for things already approved, no?

Yes on the content. Not necessarily on the "already approved". IMHO to get a conversation going, you'll need to open an issue in nodejs/node anyway. Doing some "leg work" might help with convincing you are serious, and it's not a frivolous notion.
But before that, I had an idea, since all our tests start with const common = require('../common') we could setup an overloadable common.require, then the refactor will be a simple const x = require('x') => const x = common.require('x').
Q: do you think that if we would load spdy for each require('http') in the test-http-* files, we'll get reasonable results? If so let's do a POC. That might also help with getting good feedback.

@Trott
Copy link
Member

Trott commented Mar 13, 2018

It seems like perhaps this should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that. (If the conversation indeed should continue, I suspect it may be better to have it in the main repo than here.)

@Trott Trott closed this as completed Mar 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants