Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

fix: Run expect tests on Windows with conpty pseudo-terminal#276

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
bryphe-coder merged 44 commits intomainfrombryphe/experiment/241/cross-plat-expect
Feb 15, 2022

Conversation

bryphe-coder
Copy link
Contributor

@bryphe-coderbryphe-coder commentedFeb 11, 2022
edited
Loading

This brings together a bunch of random, partially implemented packages for support of the new(ish) Windowsconpty API - such that we can leverage theexpect style of CLI tests, but in a way that works in Linux/OSXptys and Windowsconpty.

These include:

  • Vendoring thego-expect library from Netflix w/ some tweaks to work cross-platform
  • Vendoring thepty cross-platform implementation fromwaypoint-plugin-sdk
  • Vendoring theconpty Windows-specific implementation fromwaypoint-plugin-sdk
  • Adjusting thepty interface to work withgo-expect + the cross-plat version

There were several limitations with the current packages:

  • go-expect requires the sameos.File (TTY) for input / output, butconhost requires separate file handles
  • conpty does not handle input, only output
  • The cross-platformpty didn't expose the full set of primitives needed forconsole

Therefore, the following changes were made:

  • Handling ofstdin was added to theconpty interface
  • We weren't using the full extent of thego-expect interface, so some portions were removed (ie, exec'ing a process) to simplify our implementation and make it easier to extend cross-platform
  • Instead ofconsole exposing just aTty, it exposes anInTty andOutTty, to help encapsulate the difference on Windows (on Linux, these point to the same pipe)

Future improvements:

  • Theisatty implementation doesn't support accurate detection ofconhost pty's without an associated process. In lieu of a more robust check, I've added a--force-tty flag intended for test case use - that forces the CLI to run in tty mode.
  • It seems the windows implementation doesn't support setting a deadline. This is needed for the expect.Timeout API, but isn't used by us yet.

After the all-hands + meetings this afternoon - I'll get my Win environment set up to iterate to get the tests working there.

Fixes#241

kylecarbs reacted with thumbs up emojikylecarbs reacted with heart emoji
@bryphe-coderbryphe-coder self-assigned thisFeb 11, 2022
@bryphe-coderbryphe-coder changed the titleexperiment: Cross-platform (windows supported) expect testsexperiment: Cross-platform (windows supported) CLI expect testsFeb 11, 2022
@codecov
Copy link

codecovbot commentedFeb 11, 2022
edited
Loading

Codecov Report

Merging#276 (1de0f1b) intomain (64c14de) willdecrease coverage by0.26%.
The diff coverage is60.39%.

Impacted file tree graph

@@            Coverage Diff             @@##             main     #276      +/-   ##==========================================- Coverage   68.22%   67.96%   -0.27%==========================================  Files         126      130       +4       Lines        6898     7079     +181       Branches       69       69              ==========================================+ Hits         4706     4811     +105- Misses       1714     1779      +65- Partials      478      489      +11
FlagCoverage Δ
unittest-go-macos-latest66.24% <60.39%> (-0.26%)⬇️
unittest-go-ubuntu-latest67.50% <60.39%> (-0.04%)⬇️
unittest-go-windows-latest65.96% <60.39%> (?)
unittest-js65.24% <ø> (ø)
Impacted FilesCoverage Δ
cli/clitest/clitest.go93.33% <ø> (-1.91%)⬇️
expect/test_console.go0.00% <0.00%> (ø)
expect/expect.go60.37% <60.37%> (ø)
expect/console.go66.12% <66.12%> (ø)
expect/expect_opt.go72.00% <72.00%> (ø)
cli/root.go79.28% <78.57%> (-0.41%)⬇️
cli/login.go55.33% <100.00%> (+0.43%)⬆️
peerbroker/dial.go76.19% <0.00%> (-4.77%)⬇️
coderd/provisionerdaemons.go57.23% <0.00%> (-0.62%)⬇️
... and2 more

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update64c14de...1de0f1b. Read thecomment docs.

@bryphe-coderbryphe-coderforce-pushed thebryphe/experiment/241/cross-plat-expect branch from0ef4f42 to03ea70cCompareFebruary 11, 2022 21:53
@bryphe-coderbryphe-coderforce-pushed thebryphe/experiment/241/cross-plat-expect branch from95c891f toa73476dCompareFebruary 12, 2022 02:29
@bryphe-coderbryphe-coder changed the titleexperiment: Cross-platform (windows supported) CLI expect testsfix: #241 - Cross-platform (windows supported) CLI expect testsFeb 12, 2022
@bryphe-coder
Copy link
ContributorAuthor

Thanks for reviewing@kylecarbs !

@bryphe-coderbryphe-coder merged commitc9c0312 intomainFeb 15, 2022
@bryphe-coderbryphe-coder deleted the bryphe/experiment/241/cross-plat-expect branchFebruary 15, 2022 01:05
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@kylecarbskylecarbskylecarbs approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Test the coder CLI on Windows

2 participants

@bryphe-coder@kylecarbs

[8]ページ先頭

©2009-2025 Movatter.jp