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

doc,meta: add issue template for flaky tests #24450

Closed
wants to merge 2 commits into from

Conversation

refack
Copy link
Contributor

@refack refack commented Nov 17, 2018

New template for reporting flaky tests. Lists the data point relevant to later investigate the issue (i.e. CI-job, CI-worker, link to test code, console output).
Examples:
#24449
#24403
#24397

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@refack sadly an error occured when I tried to trigger a build :(

@nodejs-github-bot nodejs-github-bot added the meta Issues and PRs related to the general management of the project. label Nov 17, 2018
.github/ISSUE_TEMPLATE/4-flaky-test.md Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/4-flaky-test.md Outdated Show resolved Hide resolved
@refack
Copy link
Contributor Author

refack commented Nov 17, 2018

Anyone know how I can test this? NM
See https://github.com/refack/node/issues/new/choose

@refack refack force-pushed the add-flaky-template branch from ac7a496 to 2d5284d Compare November 17, 2018 18:27
@refack
Copy link
Contributor Author

refack commented Nov 17, 2018

BTW: this is also an option, with the long URIs out of line (👍 / 👎 your opinion)

<!--
Thank you for reporting a possible flaky test in Node.js.

Please fill in as much of the template below as you can.

-->
* test file - [NAME_OF_TEST_WITH_SUITE (e.g. parallel/test-https)][1]
* ci job  - [CI_JOB_DESCRIPTOR (e.g. aix/21613)][2]
* ci worker - [CI_WORKER_NAME](https://ci.nodejs.org/computer/CI_WORKER_NAME/)
* output:
  ``
  CI console output for the failing test.
  Or content of JUnit test details page.
  Note: should be indented with 2 spaces to align with * bullet point.
  ``

[1]: https://github.com/nodejs/node/blob/master/test/NAME_OF_TEST_WITH_SUITE.js
[2]: https://ci.nodejs.org/job/node-test-CI_JOB_DESCRIPTOR

@targos
Copy link
Member

targos commented Nov 18, 2018

/cc @Trott who opens most of those issues

@Trott
Copy link
Member

Trott commented Nov 20, 2018

/cc @Trott who opens most of those issues

I'm -0 on this.

  • I'd personally find a template to be an obstacle. I currently delete everything from the template and start entering the data.

  • I think templates should be outward facing. Adding a template for something that external folks aren't going to understand or have access to fill out will put one more choice in their path. I'd prefer they have fewer and clearer choices.

  • While it's nice to get all the info in this template, a lot of people won't know how to get some of it, and I wouldn't want that to discourage them from reporting.

Still, just a -0, not blocking or anything. If you put it in, I'll try to use it. If it causes significant problems (unlikely), we can remove it. If it helps, great.

@Trott
Copy link
Member

Trott commented Nov 20, 2018

The name/description might make people think that it's about unreliable tests they experience on their local machines (and maybe that still qualifies?) but the information requested seems to assume CI. Maybe that's OK, but maybe it's not, so I'm just pointing it out....

```
CI console output for the failing test.
Or content of JUnit test details page.
Note: should be indented with 2 spaces to align with * bullet point.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd remove this. Having to indent output by two spaces can be annoying/tedious. It's OK if it's not indented (or does that cause a big problem--seems mostly aesthetics)?

@refack
Copy link
Contributor Author

refack commented Nov 20, 2018

The name/description might make people think that it's about unreliable tests they experience on their local machines (and maybe that still qualifies?)

Yeah, that might be a good thing.

I'd personally find a template to be an obstacle. I currently delete everything from the template and start entering the data.

I'm wired to like structure, but if we don't have consensus ¯_(ツ)_/¯
Since I'm probably the second most frequent opener of these, I can keep mine consistent.

@refack refack closed this Dec 21, 2018
@refack refack deleted the add-flaky-template branch December 21, 2018 01:12
@refack refack restored the add-flaky-template branch January 22, 2019 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants