Skip to content

flake: scaletest/workspacetraffic TestRun/RPTY #11175

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

Closed
spikecurtis opened this issue Dec 13, 2023 · 2 comments · Fixed by #11633
Closed

flake: scaletest/workspacetraffic TestRun/RPTY #11175

spikecurtis opened this issue Dec 13, 2023 · 2 comments · Fixed by #11633
Assignees

Comments

@spikecurtis
Copy link
Contributor

Seen on https://github.com/coder/coder/runs/19555780615

    run_test.go:136: read errors: 0
    run_test.go:137: write errors: 0
    run_test.go:138: bytes read total: 5464
    run_test.go:139: bytes written total: 5120
    run_test.go:142: 
        	Error Trace:	/home/runner/actions-runner/_work/coder/coder/scaletest/workspacetraffic/run_test.go:142
        	Error:      	Max difference between 1024 and 5120 allowed is 0.1, but difference was -4096
        	Test:       	TestRun/RPTY
@mafredri
Copy link
Member

Looks like first there’s a 5s delay establishing the connection (normal), but also, screen is being used which is unexpected.

I think that’s the most likely culprit here as screen can spit out extra data or even re-render in some cases (doubt that’s what’s happening though).

Maybe we add a way to disable screen, even when it’s present. We could add backend support to the init payload, for example. Or maybe we can make the agent ignore screen in the test some other way. Thoughts/preference here @code-asher?

2023-12-12T12:29:54.1530623Z     t.go:84: 2023-12-12 12:29:43.234 [debu]  agent.reconnecting-pty: spawning screen client  remote=[fd7a:115c:a1e0:4d37:ab3e:19ab:e49b:c41]:33297  local=[fd7a:115c:a1e0:4bbe:9a01:8a64:2780:efac]:2  message_id=13a71e00-e630-47ac-85f7-094c6002e620  connection_id=1e1ceed0-3a78-464a-b646-90e2980671a8  screen_id=0c5e10fa
2023-12-12T12:29:54.1532423Z     t.go:84: 2023-12-12 12:29:43.456 [debu]  agent.reconnecting-pty: unable to read pty output; screen might have exited  remote=[fd7a:115c:a1e0:4d37:ab3e:19ab:e49b:c41]:33297  local=[fd7a:115c:a1e0:4bbe:9a01:8a64:2780:efac]:2  message_id=13a71e00-e630-47ac-85f7-094c6002e620  connection_id=1e1ceed0-3a78-464a-b646-90e2980671a8  error=EOF
2023-12-12T12:29:54.1534257Z     t.go:84: 2023-12-12 12:29:43.456 [debu]  agent.reconnecting-pty: killed process with error  remote=[fd7a:115c:a1e0:4d37:ab3e:19ab:e49b:c41]:33297  local=[fd7a:115c:a1e0:4bbe:9a01:8a64:2780:efac]:2  message_id=13a71e00-e630-47ac-85f7-094c6002e620  connection_id=1e1ceed0-3a78-464a-b646-90e2980671a8  error="os: process already finished"

@matifali matifali added bug and removed chore labels Dec 13, 2023
@code-asher
Copy link
Member

It could be the startup message that appears in the bottom left corner that is causing the extra bytes.

My first draft actually did have a backend parameter, we could bring that back. But, it feels a bit wrong to me because really I think the other backend method should probably not exist given the bugs it has. Crazy it can input random characters and drop modes, among other issues. 😆

I think my preferences in order would be:

  1. Adjust the test to account for the message (maybe we can allow either 5120 or 1024?)
  2. See if we can eliminate the message (I am not sure there is a way)
  3. Replace both backends with something else (something that does xterm-compatible terminal emulation)
  4. Disable screen for the test

If we need to disable screen, even if temporarily, in the buffer backend tests we disable screen by setting PATH. But if that will not work here then a backend parameter seems like the way to go.

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 a pull request may close this issue.

4 participants