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

test: purge stale disabled tests #2045

Closed
wants to merge 1 commit into from
Closed

Conversation

Trott
Copy link
Member

@Trott Trott commented Jun 23, 2015

There are many tests in the test/disabled directory. These tests are not used by Makefile nor by the CI. Other than a single 2015 commit that puts 'use strict' in each test, many of them haven't been touched in years.

This removes all the disabled tests that have been unmodified since 2011 (with the exception of the 'use strict' modification mentioned above).

I concede in advance that my main motive is a perhaps-misguided mission to eliminate every TODO comment possible in the code base.

@Trott Trott added the test Issues and PRs related to the tests. label Jun 23, 2015
@bnoordhuis
Copy link
Member

LGTM. I don't see a reason to keep them around. Can you put your rationale in the commit log?

@brendanashworth
Copy link
Contributor

sweet, they're in git anyways so I'm not sure why they were kept in the first place

Tests in the disabled directory are not used by Makefile nor by the CI.
Other than a single 2015 commit that puts 'use strict' in each test,
many of them haven't been touched in years.

This removes all the disabled tests that have been unmodified since
2011 (with the exception of the 'use strict' modification mentioned
above).
@Trott
Copy link
Member Author

Trott commented Jun 23, 2015

Commit message amended per @bnoordhuis

@evanlucas
Copy link
Contributor

LGTM

@bnoordhuis
Copy link
Member

they're in git anyways so I'm not sure why they were kept in the first place

We (Ryan, Bert, Igor, me) moved a lot of tests to test/disabled when we were porting node.js to Windows, piecemeal moving them back whenever libuv grew the required functionality.

What's left are tests for functionality that was too UNIX-specific or too libev/libeio-centric.

Trott added a commit to Trott/io.js that referenced this pull request Jun 25, 2015
Tests in the disabled directory are not used by Makefile nor by the CI.
Other than a single 2015 commit that puts 'use strict' in each test,
many of them haven't been touched in years.

This removes all the disabled tests that have been unmodified since
2011 (with the exception of the 'use strict' modification mentioned
above).

PR-URL: nodejs#2045
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@Trott
Copy link
Member Author

Trott commented Jun 25, 2015

Merged in 856c11f

@Trott Trott closed this Jun 25, 2015
@rvagg rvagg mentioned this pull request Jun 30, 2015
mscdex pushed a commit to mscdex/io.js that referenced this pull request Jul 9, 2015
Tests in the disabled directory are not used by Makefile nor by the CI.
Other than a single 2015 commit that puts 'use strict' in each test,
many of them haven't been touched in years.

This removes all the disabled tests that have been unmodified since
2011 (with the exception of the 'use strict' modification mentioned
above).

PR-URL: nodejs#2045
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Evan Lucas <evanlucas@me.com>
@Trott Trott deleted the disabled-cleanup branch January 9, 2022 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants