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: disable fs watch tests for AIX #5187

Closed
wants to merge 1 commit into from
Closed

Conversation

mhdawson
Copy link
Member

fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but uses it as part of what it is testing so its included in
those disabled.

@mhdawson mhdawson added the test Issues and PRs related to the tests. label Feb 10, 2016
# regressions until this work is complete
[$system==aix]
test-fs-watch-enoent : FAIL, PASS
test-async-wrap-check-providers : FAIL, PASS
Copy link
Member

Choose a reason for hiding this comment

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

It might be better with test-async-wrap-check-providers to handle it in the test itself, otherwise you might be missing out on some real regressions (it's a test that tests a number of subsystems, fs.watch() is just one of them.)

If you do, you should note it in the comment here so it's not forgotten about.

@bnoordhuis
Copy link
Member

Left a suggestion but LGTM if you don't want to take it.

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Feb 10, 2016
@mhdawson
Copy link
Member Author

Good point, I'll take a look at test-async-wrap-check-providers to look at just excluding the required part.

fs watch currently needs special configuration on AIX and we
want to improve under nodejs#5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX
@mhdawson
Copy link
Member Author

@bnoordhuis updated

@mhdawson mhdawson self-assigned this Feb 11, 2016
@jasnell
Copy link
Member

jasnell commented Feb 13, 2016

LGTM

@mhdawson
Copy link
Member Author

mhdawson added a commit that referenced this pull request Feb 16, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mhdawson
Copy link
Member Author

landed as 6a85ad9

@mhdawson
Copy link
Member Author

CI run to confirm on AIX: https://ci.nodejs.org/job/node-test-commit-aix/46/

@mhdawson
Copy link
Member Author

confirmed we are down to the 1 unrelated failure in the AIX runs, closing

@mhdawson mhdawson closed this Feb 16, 2016
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg pushed a commit that referenced this pull request Feb 18, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Feb 18, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@MylesBorins MylesBorins mentioned this pull request Feb 18, 2016
stefanmb pushed a commit to stefanmb/node that referenced this pull request Feb 23, 2016
fs watch currently needs special configuration on AIX and we
want to improve under nodejs#5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: nodejs#5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this pull request Mar 2, 2016
fs watch currently needs special configuration on AIX and we
want to improve under #5085.
Tests are disabled so CI can be green and we can spot other
regressions until this work is complete.

test-async-wrap-check-providers does not aim to test fs watch
but part of the test uses it so that part has been skipped for
AIX

PR-URL: #5187
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@mhdawson mhdawson deleted the aixfs branch March 15, 2017 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants