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

Should there be a Testing WG? #3872

Closed
Trott opened this issue Nov 17, 2015 · 40 comments
Closed

Should there be a Testing WG? #3872

Trott opened this issue Nov 17, 2015 · 40 comments
Labels
discuss Issues opened for discussions and feedbacks. test Issues and PRs related to the tests.

Comments

@Trott
Copy link
Member

Trott commented Nov 17, 2015

Would it be useful to have a Testing WG? Things a WG might do:

  • OMG fix all those flaky tests.
  • Create some guidelines around tests. (For example, when should a file containing multiple tests be split into multiple files?)
  • Maybe work with the Build WG on the issue of buildbot failures, although I suspect the Build WG is all over that problem. (But just in case: Is there a way to test for buildbot failures? Automate notification to relevant parties so they can be fixed promptly? Is there a way to automate retry when a buildbot fails? Maybe a Test WG can be given limited privileges on the CI infrastructure such that the tedious/mechanistic elements of dealing with buildbot failures can be delegated to them?)
  • Maybe see about porting as much of the Python test harness code to Node as is sensible.

Anyone interested?

@Trott Trott added discuss Issues opened for discussions and feedbacks. test Issues and PRs related to the tests. labels Nov 17, 2015
@Trott
Copy link
Member Author

Trott commented Nov 17, 2015

Oh, and if the Smoke Test WG never got off the ground, maybe there would be interest from a Testing WG to work on citgm.

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

Thus far @thealphanerd has been working on evolving citgm and has been making great progress but it would always be great to have additional people. I'm not sure we need yet another WG as much as we just need more people working on it. There have been a ton of test fixes landed here lately and it's great to see.

@cjihrig
Copy link
Contributor

cjihrig commented Nov 17, 2015

I think putting smoke testing under a more general testing WG would make sense.

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

Ref: #2317

@MylesBorins
Copy link
Contributor

Maybe something akin to Tooling?

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

I do think a Tooling WG would be very interesting

@MylesBorins
Copy link
Contributor

Seems like a lot of technical challenges to various wg's could fall in here.

Although it may overlap with @nodejs/build a bit

@cjihrig
Copy link
Contributor

cjihrig commented Nov 17, 2015

I agree. The line between build and tooling would be very blurry.

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

build tends to focus specifically on the build toolchain and CI. The kind of tooling I'm thinking would include tools like Rod's iojs-tools, citgm, eslint updates, that sort of thing. Thinking about it tho, that doesn't really put enough focus on the testing aspect. hmm

@jbergstroem
Copy link
Member

@jasnell people in build are pretty diverse in how time is spent. Sitting close to all vm's you tend to look at/attempt to fix failed tests, improve tooling (both for the build environment, jenkins and node in general), security, releases and so on. For me, a WG responsible for tests and conformance would make most sense. Stuff like this would suit perfectly in there.

@jasnell
Copy link
Member

jasnell commented Nov 17, 2015

Right on. OK, I'm on board :) let's do it!
On Nov 17, 2015 2:35 PM, "Johan Bergström" notifications@github.com wrote:

@jasnell https://github.com/jasnell people in build are pretty diverse
in how time is spent. Sitting close to all vm's you tend to look at/attempt
to fix failed tests, improve tooling (both for the build environment,
jenkins and node in general), security, releases and so on. For me, a WG
responsible for tests and conformance would make most sense. Stuff like
this nodejs/build#248 would suit perfectly in
there.


Reply to this email directly or view it on GitHub
#3872 (comment).

@joaocgreis
Copy link
Member

@Trott regarding your initial comment, we do have a tool to send us an e-mail when buildbots fail, needless to say we had a lot of activity last week. We are tracking the failures in nodejs/build#232 , please call it to our attention if you see any random disconnect. Thanks for the great work solving flaky tests!

@orangemocha
Copy link
Contributor

This sounds like a good idea to me. In the modern days' philosophy of "devs write the tests", I think there is something to be missed from having a dedicated group of people focused on how to improve the quality of the product through testing.

Certainly there is some overlap with the current Build WG responsibilities (especially on the test infra) but that doesn't need to be a problem. And if collaborators want to help improve the test infrastructure, they are welcome to do so regardless of which working groups they belong to 😄

OMG fix all those flaky tests.

I do think this might need to become the responsibility of all collaborators. If a testing WG helps getting traction on flaky tests, that's great. Please join the discussion at nodejs/build#248

Luckily, the buildbot disconnects seem to be fixed.

Adding a few things to the list of things this group might work on:

  • Analyze code coverage
  • Write stress tests

@jasnell
Copy link
Member

jasnell commented Nov 18, 2015

  • Review and Improve existing tests

@mikeal
Copy link
Contributor

mikeal commented Nov 18, 2015

At the very least it sounds like we should have a testing team so we can bring them in to review test related PRs, similar to what we have for the crypto team.

@jbergstroem
Copy link
Member

I agree with @orangemocha that all devs should care about fails, especially if you cause them. What flaky tests has shown us is that it's not quite that simple. A fail might occur on an operating system the dev don't have access to, or it's "flaky" enough to only occur a few times out of a thousand runs. It quickly slips out of mind.

With @joaocgreis's new stress tester tooling just got better, but having a test WG who has "zero fails" on its agenda with people actively working against that goal is awesome. I'd also like to see an overhaul of the test runner at some point, but that's probably something I should join the WG and drive myself :)

@rsp
Copy link
Contributor

rsp commented Dec 2, 2015

I wonder how is the Testing WG idea going and whether it could result in some higher level test guidelines. Something like:

  • tests shouldn't fail if there is no problem with node
  • failed tests should mean failure of the build process
  • ./configure && make && make test && make install should work by default
  • failed tests should produce informative output instead of stack traces

and things like that.

Right now here is my experience of building node on stock Ubuntu:
I run ./configure && make && make test and I get output like this:

=== release test-require-long-path ===                                         
Path: parallel/test-require-long-path
fs.js:842
  return binding.mkdir(pathModule._makeLong(path),
                 ^

Error: ENAMETOOLONG: name too long, mkdir '/home/rsp/node/node-v5.1.0/test/tmp.2/xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
    at Error (native)
    at Object.fs.mkdirSync (fs.js:842:18)
    at Object.<anonymous> (/home/rsp/node/node-v5.1.0/test/parallel/test-require-long-path.js:16:4)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:313:12)
    at Function.Module.runMain (module.js:457:10)
    at startup (node.js:138:18)
    at node.js:974:3
Command: out/Release/node /home/rsp/node/node-v5.1.0/test/parallel/test-require-long-path.js
[01:05|% 100|+ 967|-   2]: Done                                                
make: *** [test] Error 1

It means that this test doesn't work on ecryptfs and I need to build it in /tmp instead of /home to make tests pass, which may not be clear to most of people who see that tests fail and have an impression that their build is broken and they shouldn't run make install.

I made three PRs #3925, #3929 and #4116 that fix this specific problem in different ways that were suggested by different people. Those are my first contributions to Node and it is not clear to me who should I listen to or what general philosophy should I follow. Not having any clear guidelines for tests I can only prepare few solutions to the same problem in hope that one of them finally gets accepted.

I hope that having a test WG would help in making the tests consistent, meaning not only their output but also preferred ways to deal with issues like those in the PRs above to avoid bikeshedding in simple cases as the one I described.

I don't think I would be experienced enough to join such a WG but I will be happy to help with writing such guidelines that would be accepted by the community and trying to make sure the tests follow them.

So, 👍 for the Testing WG.

@jasnell
Copy link
Member

jasnell commented Dec 2, 2015

@mikeal ...a testing team seems like a good place to start. Who should be the initial members, I'll get the team created.

@jasnell
Copy link
Member

jasnell commented Dec 2, 2015

Ok, @nodejs/testing created. Initial set of members include @orangemocha, @jbergstroem, @thealphanerd, @cjihrig, @Trott and myself.

@Trott
Copy link
Member Author

Trott commented Dec 2, 2015

Would it make sense to create a team repo too (say, nodejs/testing) so that we can have issues and PRs for meetings, meeting notes, stuff we're doing....

@joaocgreis
Copy link
Member

I might not be a big help in solving all the flaky tests, but count on me for windows specific failures as time permits. I'll be maintaining the Jenkins jobs to help as much as possible. @jasnell feel free to add me if it makes sense.

@jbergstroem
Copy link
Member

I'm going to respectfully back out of this WG -- reckon I'll be involved pretty closely through the build group anyway.

@Trott
Copy link
Member Author

Trott commented Dec 17, 2015

Since @jbergstroem has said that he would prefer to not add this WG to his plate and @joaocgreis has expressed a willingness, can someone with ownership privileges (@jasnell? @cjihrig?) make that change?

@orangemocha
Copy link
Contributor

Added @joaocgreis and removed @jbergstroem

@Trott
Copy link
Member Author

Trott commented Jan 2, 2016

Probably makes sense to ask @mscdex and @santigimeno if either of them would be interested in participating in a Testing WG. Oh, look, I just @ mentioned them so now they've been asked.

@jbergstroem
Copy link
Member

@santigimeno has been great help so far! Please join :)

@Trott
Copy link
Member Author

Trott commented Jan 2, 2016

I've put together a draft of WG formation docs like a charter etc. Feedback welcome, of course. (Would weekly meetings be too often? Do we need to meet at all?)

Moreover, we should probably start Doing The Work before asking to be ratified, so maybe the next step is to put together a Doodle poll to figure out when people can meet (or to decide that meetings isn't how this WG is going to work).

@mscdex
Copy link
Contributor

mscdex commented Jan 2, 2016

I'm not sure how much time I'd be able to devote in/to a Testing WG, I just really like to see green CI statuses when there are no failures so that's why I had been trying to fix flaky tests recently :-)

@santigimeno
Copy link
Member

I would be indeed interested. Thanks for asking :)

@Trott
Copy link
Member Author

Trott commented Jan 3, 2016

Someone with owner privs want to add santigimeno to nodejs/testing?

@Trott
Copy link
Member Author

Trott commented Jan 3, 2016

I've set up a Doodle poll to try to figure out what might be a good time to have our first meeting via Google Hangout. http://doodle.com/poll/cccu446299gae5d5 http://doodle.com/poll/c24vvkga3zfrrhn8 /cc @nodejs/testing @santigimeno

@orangemocha
Copy link
Contributor

Invited santigimeno to nodejs/testing.

@orangemocha
Copy link
Contributor

@Trott I filled the doodle, though it didn't show which time zone it is on. In the time proposals page, you need to enable time zone support, I think.

It might also be better to give more advance notice for the meeting dates so that everyone has the chance to respond and agree on a time.

@Trott
Copy link
Member Author

Trott commented Jan 5, 2016

@orangemocha Alas, the "allow more advance notice" thing is hard to argue with. I'm going to cancel this Doodle poll and set up another one with dates slightly further out.

@Trott
Copy link
Member Author

Trott commented Jan 5, 2016

OK, attempt number 2 at a poll for a meeting time. Meeting times are all at least one week out from now and I enabled time zone support so it will hopefully show you the time based on your time zone. I also tried to have a reasonable spread of times to avoid a "all these times are in the middle of the night in my time zone" issues.

Here's the link, please fill it out! Thanks! http://doodle.com/poll/c24vvkga3zfrrhn8

@Trott
Copy link
Member Author

Trott commented Jan 7, 2016

Poll closed! Only time that was available for every single respondent is the time we're going with:

Friday, January 15, 10AM Pacific Time

More soon...

Hey, if someone with org privs wants to set up a testing repo, I could move this sort of organizational stuff in there and close this issue. :-)

@orangemocha
Copy link
Contributor

@jasnell
Copy link
Member

jasnell commented Jan 8, 2016

@Trott ... are you able to send out a regular calendar invite for the meeting?

@Trott
Copy link
Member Author

Trott commented Jan 8, 2016

@jasnell When I set up a Google Hangout On Air, I believe it should send a calendar invite...

@Trott
Copy link
Member Author

Trott commented Jan 8, 2016

@orangemocha Thanks! I'll shift everything over to that repo and close this issue.

@Trott Trott closed this as completed Jan 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issues opened for discussions and feedbacks. test Issues and PRs related to the tests.
Projects
None yet
Development

No branches or pull requests