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

Add basic tests for encoding option with PtyFork. #65

Merged
merged 2 commits into from
Mar 30, 2017

Conversation

KiNgMaR
Copy link

@KiNgMaR KiNgMaR commented Mar 27, 2017

Hi @Tyriar,

here are the promised test cases for the encoding option. I only added tests for the pty fork part.

I also started adding them for the pty open part, but now I'm not sure whether having the encoding option there even makes sense. Can you maybe elaborate on how a terminal created with pty open would be used from another process? I can only think of redirection to /dev/pts/123 or similar? Is that commonly used?

Thanks...!

@Tyriar
Copy link
Member

Tyriar commented Mar 29, 2017

Thanks @KiNgMaR, I'll check this out when I get past all the release stuff at work 😃

@Tyriar Tyriar added this to the 0.7.0 milestone Mar 29, 2017
@Tyriar Tyriar self-assigned this Mar 29, 2017
@Tyriar Tyriar self-requested a review March 29, 2017 20:48
@Tyriar
Copy link
Member

Tyriar commented Mar 30, 2017

@KiNgMaR I fixed up the build in #66 then merge in master to your branch, the tests seem to be failing though. You probably know why better than me 😃

https://travis-ci.org/Tyriar/node-pty/jobs/216624074

- raise mocha timeout because sometimes launching a shell is slow.
- use separate files for UTF-8 test data to avoid issues with host shell
configuration.
@Tyriar
Copy link
Member

Tyriar commented Mar 30, 2017

Thanks for the follow up @KiNgMaR, should ensure no regressions 😄

@Tyriar Tyriar merged commit 3e46104 into microsoft:master Mar 30, 2017
@Tyriar Tyriar modified the milestones: 0.6.4, 0.7.0 Jul 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants