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.

Sign up for GitHub

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

Jump to bottom

[backport v10.x] test: fix pty test hangs on aix #28826

Closed
wants to merge 3 commits into from
Closed

[backport v10.x] test: fix pty test hangs on aix #28826

wants to merge 3 commits into from

Commits on Jul 24, 2019

  1. test: skip tests related to CI failures on AIX 

    These tests seem to trigger failures in the entire CI job (not just the
test) on AIX. Skip them to see if that helps alleviate spurious failures
in node-test-commit-aix (and the upstream PR and commit test jobs).

See:
- nodejs/build#1820 (comment)
- nodejs/build#1847 (comment)

PR-URL: nodejs#28469
Backport-PR-URL: nodejs#28826
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
    @sam-github @Trott
    sam-github authored and Trott committed Jul 24, 2019
    Configuration menu
    Copy the full SHA
    5412a62 View commit details
    Browse the repository at this point in the history

  2. test: skip stringbytes-external-exceed-max on AIX 

    Add SKIP status for more tests in stringbytes-external-exceed-max that
are failing on AIX.

PR-URL: nodejs#28516
Backport-PR-URL: nodejs#28826
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
    @sam-github @Trott
    sam-github authored and Trott committed Jul 24, 2019
    Configuration menu
    Copy the full SHA
    e0cbe72 View commit details
    Browse the repository at this point in the history

  3. test: fix pty test hangs on aix 

    Some pty tests persistently hung on the AIX CI buildbots. Fix that by
adding a helper script that properly sets up the pty before spawning
the script under test.

On investigation I discovered that the test runner hung when it tried
to close the slave pty's file descriptor, probably due to a bug in
AIX's pty implementation. I could reproduce it with a short C program.
The test runner also leaked file descriptors to the child process.

I couldn't convince python's `subprocess.Popen()` to do what I wanted
it to do so I opted to move the logic to a helper script that can do
fork/setsid/etc. without having to worry about stomping on state in
tools/test.py.

In the process I also uncovered some bugs in the pty module of the
python distro that ships with macOS 10.14, leading me to reimplement
a sizable chunk of the functionality of that module.

And last but not least, of course there are differences between ptys
on different platforms and the helper script has to paper over that.
Of course.

Really, this commit took me longer to put together than I care to admit.

Caveat emptor: this commit takes the hacky ^D feeding to the slave out
of tools/test.py and puts it in the *.in input files. You can also feed
other control characters to tests, like ^C or ^Z, simply by inserting
them into the corresponding input file. I think that's nice.

Fixes: nodejs/build#1820
Fixes: nodejs#28489

PR-URL: nodejs#28600
Backport-PR-URL: nodejs#28826
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
    @bnoordhuis @Trott
    bnoordhuis authored and Trott committed Jul 24, 2019
    Configuration menu
    Copy the full SHA
    dc4b64f View commit details
    Browse the repository at this point in the history