pty: change the way we interact with ptys

[paul-marechal]

The hack using sockets was causing troubles where some lines were not correctly sent. This commit tries to simplify the pty fd handling.

There is an issue when opening both the master and slave ends from the same process on Node though. When doing so Node will hang at exit because of stuck read calls in the eventloop. I wasn't able to figure out the cause, so a hack I used was to open the slave end into a subprocess that we kill once done with it. This only impacts our test suite.

Signed-off-by: Paul Maréchal paul.marechal@ericsson.com

[paul-marechal]

run tests

[jonahgraham]

run tests

[jonahgraham]

@marechal-p I added you to the admin list, but because you were already on the whitelist I am not sure why the build didn't run: https://ci.eclipse.org/cdt/view/npm%20builds/job/cdt-gdb-adapter-verify/configure

[jonahgraham]

The tests are failing with this change https://ci.eclipse.org/cdt/job/cdt-gdb-adapter-verify/131/console

I re-ran master (https://ci.eclipse.org/cdt/view/npm%20builds/job/cdt-gdb-adapter-master/137/) just to make sure that master was working fine.

22:37:54  $ cross-env JUNIT_REPORT_PATH=test-reports/native.xml JUNIT_REPORT_STACK=1 JUNIT_REPORT_PACKAGES=1 mocha --reporter mocha-jenkins-reporter dist/native/*.spec.js
22:37:54  
22:37:54    pty creation
22:37:54        ✓ should be able to open a ptmx/pts pair: 29ms
22:37:54        1) "after each" hook
22:37:54  
22:37:54    Suite duration: 0.033 s, Tests: 2
22:37:54  
22:37:54    1 passing (35ms)
22:37:54    1 failing
22:37:54  
22:37:54    1) pty creation
22:37:54         "after each" hook:
22:37:54       Uncaught Error: EBADF: bad file descriptor, close
22:37:54    
22:37:54  
22:37:54  
22:37:54  
22:37:54  
22:37:54    1 passing (35ms)
22:37:54    1 failing
22:37:54  
22:37:54    1) pty creation
22:37:54         "after each" hook:
22:37:54       Uncaught Error: EBADF: bad file descriptor, close
22:37:54    

It may also be hung, if the timeout doesn't stop the build, in the morning I'll look into that.

[paul-marechal]

Thanks for the comments! I will look into both the error and the process hanging.

[paul-marechal]

Oddly enough it passes on my machine.

[paul-marechal]

run tests

[paul-marechal]

I managed to fix the hanging issue. It is a bit hackish, but I couldn't figure out why it hanged without the hack. See commit message for more details.

[jonahgraham]

Sorry to be bearer of bad news :-( The tests are failing on the build machine in a new way now:

23:34:17  $ cross-env JUNIT_REPORT_PATH=test-reports/native.xml JUNIT_REPORT_STACK=1 JUNIT_REPORT_PACKAGES=1 mocha --reporter mocha-jenkins-reporter dist/native/*.spec.js
23:34:17  
23:34:17    pty creation
23:34:17        1) should be able to open a ptmx/pts pair
23:34:17  
23:34:17    Suite duration: 0.026 s, Tests: 1
23:34:17  
23:34:17    0 passing (28ms)
23:34:17    1 failing
23:34:17  
23:34:17    1) pty creation
23:34:17         should be able to open a ptmx/pts pair:
23:34:17       TypeError: stream_1.EventEmitter is not a constructor
23:34:17        at new ForkedFile (dist/native/forked-file.js:32:23)
23:34:17        at Context.<anonymous> (dist/native/pty.spec.js:41:25)
23:34:17        at Generator.next (<anonymous>)
23:34:17        at /home/jenkins/agent/workspace/cdt-gdb-adapter-verify/dist/native/pty.spec.js:17:71
23:34:17        at new Promise (<anonymous>)
23:34:17        at __awaiter (dist/native/pty.spec.js:13:12)
23:34:17        at Context.<anonymous> (dist/native/pty.spec.js:39:20)
23:34:17        at Context.<anonymous> (dist/native/pty.spec.js:79:26)
23:34:17  

Let me know if I can do anything to help.

PS In case you haven't seen this: there are instructions for running tests in the docker container, useful for times when things pass locally, but fail on build machine https://github.com/eclipse-cdt/cdt-gdb-adapter/blob/master/src/integration-tests/README.md#running-the-tests-using-docker

[paul-marechal]

Fixed the wrong import. I will look into your link for next time, thanks!

[paul-marechal]

CI is green \o/

[paul-marechal]

I followed your instructions and I managed to run the suite locally which passed. Thanks!

This PR is ready for review.

[ruspl-afed]

I am surprised that my previous approval was not nullified by the new commits so I manually redid it (part of learning github flow I suppose).

I believe this is configurable by repository admin

[jonahgraham]

I believe this is configurable by repository admin

Thanks - I raised https://bugs.eclipse.org/bugs/show_bug.cgi?id=570100 to change this setting

[paul-marechal]

Thanks for the review! I wanted to make extra sure that the issues we were seeing were fixed. Unfortunately I wasn't able to come up with a test to reproduce in this repo.