UnitTest failures on main

[jlewi]

Running on main I'm seeing test failures.

git log
commit 61fc8785ab8adb4cec5f1135d278a9e8895f327c (HEAD, upstream/main, upstream/HEAD)
Author: Sebastian Tiedtke <sebastiantiedtke@gmail.com>
Date:   Thu May 23 16:41:17 2024 -0400

    Is python3 more likely than python? (#586)

    `python` is an alias for `python3` on my BMP which makes the test fail.

Here are the test failures

 logger.go:146: 2024-05-23T23:09:03.531Z    INFO    stream canceled after the process finished; ignoring    {"id": "01HYKVE2NV87J6ZQTMY69E6BYT"}
    --- FAIL: TestRunnerServiceServerExecute_WithInput/SimulateCtrlC (1.52s)
        service_execute_test.go:643: 
                Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:643
                Error:          "rpc error: code = Unknown desc = exit status 1" does not contain "exit status 130"
                Test:           TestRunnerServiceServerExecute_WithInput/SimulateCtrlC
        service_execute_test.go:644: 
                Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:644
                Error:          Not equal: 
                                expected: 130
                                actual  : 1
                Test:           TestRunnerServiceServerExecute_WithInput/SimulateCtrlC

    --- FAIL: TestRunnerServiceServerExecute_WithInput/ContinuousInput (0.04s)
        service_execute_test.go:600: 
                Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:600
                Error:          Not equal: 
                                expected: "a\r\nb\r\nc\r\nd\r\nA\r\nB\r\nC\r\nD\r\n"
                                actual  : "a\r\nb\r\nA\r\nB\r\nc\r\nC\r\nd\r\nD\r\n"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -2,7 +2,7 @@
                                 b
                                -c
                                -d
                                 A
                                 B
                                +c
                                 C
                                +d
                                 D
                Test:           TestRunnerServiceServerExecute_WithInput/ContinuousInput

2024-05-23T23:09:03.740Z    DEBUG   runner/command.go:404   finished copying from pty to stdout {"count": 32}
--- FAIL: Test_command (0.00s)
    --- FAIL: Test_command/JavaScript (0.03s)
        command_test.go:107: 
                Error Trace:    /Users/jlewi/git_runme/internal/runner/command_test.go:107
                Error:          Not equal: 
                                expected: ""
                                actual  : "(node:24760) ExperimentalWarning: The Node.js specifier resolution flag is experimental. It could change or be removed at any time.\n(Use `node --trace-warnings ...` to show where the warning was created)\n"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1,3 @@
                                +(node:24760) ExperimentalWarning: The Node.js specifier resolution flag is experimental. It could change or be removed at any time.
                                +(Use `node --trace-warnings ...` to show where the warning was created)

                Test:           Test_command/JavaScript
    --- FAIL: Test_command/JavaScriptEnv (0.03s)
        command_test.go:135: 
                Error Trace:    /Users/jlewi/git_runme/internal/runner/command_test.go:135
                Error:          Not equal: 
                                expected: ""
                                actual  : "(node:24765) ExperimentalWarning: The Node.js specifier resolution flag is experimental. It could change or be removed at any time.\n(Use `node --trace-warnings ...` to show where the warning was created)\n"

                                Diff:
                                --- Expected
                                +++ Actual
                                @@ -1 +1,3 @@
                                +(node:24765) ExperimentalWarning: The Node.js specifier resolution flag is experimental. It could change or be removed at any time.
                                +(Use `node --trace-warnings ...` to show where the warning was created)

                Test:           Test_command/JavaScriptEnv
FAIL
FAIL    github.com/stateful/runme/v3/internal/runner    4.624s
ok      github.com/stateful/runme/v3/internal/runner/client 0.255s
--- FAIL: TestRunnerServiceServerExecute_WithInput (1.61s)

Tests appear to have passed on main at this commit #586 .

Are others seeing test failures when running locally or is it just me?

[jlewi]

It looks like a bunch of tests including some of the ones the are failing aren't running in GHA.

There are two test steps in CI. The first is test on docker https://github.com/stateful/runme/blob/61fc8785ab8adb4cec5f1135d278a9e8895f327c/.github/workflows/ci.yml#L60

This ends up running tests with tag test_with_docker

TZ=UTC go test -ldflags="-X 'github.com/stateful/runme/v3/internal/version.BuildVersion=3.3.1-3-g61fc878-61fc878'" -run=".*" -tags="test_with_docker" -timeout=90s -covermode=atomic -coverprofile=cover.out -coverpkg=./... "./..."

So only a subset of tests get run. TestRunnerServiceServerExecute_WithInput/SimulateCtrlC for example is not included.

There is a CI step to run tests on windows https://github.com/stateful/runme/blob/61fc8785ab8adb4cec5f1135d278a9e8895f327c/.github/workflows/ci.yml#L67

This doesn't include any tags to restrict the tests. However, a bunch of tests are configured not to get built on windows. e.g https://github.com/stateful/runme/blob/61fc8785ab8adb4cec5f1135d278a9e8895f327c/internal/runnerv2service/service_execute_test.go#L1

So it looks like we may have tests running locally that don't run on CI.

[jlewi]

Here's my proposal

1. Create a new GHA step that runs tests with no tag constraints.

2. Skip any tests which are currently failing using a skip directive e.g.

   
    if !testutil.runFlaky() {
       t.Skipf("Skipping flaky test")
    }

3. Open up individual issues to resolve specific failing tests

If this sounds good I can take this on; @sourishkrout let me know.

[sourishkrout]

😮 this is actually a bug in how the vscode terminal connects back to the kernel server running inside of vscode. The env vars seem to be persistent when reloaded/revived. I can repro the issue. Attempting a fix here: https://github.com/stateful/vscode-runme/pull/1379

 logger.go:146: 2024-05-23T23:09:03.531Z    INFO    stream canceled after the process finished; ignoring    {"id": "01HYKVE2NV87J6ZQTMY69E6BYT"}
    --- FAIL: TestRunnerServiceServerExecute_WithInput/SimulateCtrlC (1.52s)
        service_execute_test.go:643: 
                Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:643
                Error:          "rpc error: code = Unknown desc = exit status 1" does not contain "exit status 130"
                Test:           TestRunnerServiceServerExecute_WithInput/SimulateCtrlC
        service_execute_test.go:644: 
                Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:644
                Error:          Not equal: 
                                expected: 130
                                actual  : 1
                Test:           TestRunnerServiceServerExecute_WithInput/SimulateCtrlC

[adambabik]

Thanks for reporting!

• TestRunnerServiceServerExecute_WithInput/SimulateCtrlC -- it's flaky. One possible solution is to use a loop to send signals and validate responses. Sometimes input characters are sent too fast.
• TestRunnerServiceServerExecute_WithInput/ContinuousInput -- this is quite interesting. That output is completely valid and it's race condition.
• Test_command/JavaScript and Test_command/JavaScriptEnv are failing due to different node.js version. We should run these only in the CI as locally it does not make sense. It kinda worked so far because all contributors have had the same node.js version. We can hide it with a build tag similarly to tests that expect Docker.

[sourishkrout]

😮 this is actually a bug in how the vscode terminal connects back to the kernel server running inside of vscode. The env vars seem to be persistent when reloaded/revived. I can repro the issue. Attempting a fix here: stateful/vscode-runme#1379

 logger.go:146: 2024-05-23T23:09:03.531Z  INFO    stream canceled after the process finished; ignoring    {"id": "01HYKVE2NV87J6ZQTMY69E6BYT"}
    --- FAIL: TestRunnerServiceServerExecute_WithInput/SimulateCtrlC (1.52s)
        service_execute_test.go:643: 
              Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:643
              Error:          "rpc error: code = Unknown desc = exit status 1" does not contain "exit status 130"
              Test:           TestRunnerServiceServerExecute_WithInput/SimulateCtrlC
        service_execute_test.go:644: 
              Error Trace:    /Users/jlewi/git_runme/internal/runnerv2service/service_execute_test.go:644
              Error:          Not equal: 
                              expected: 130
                              actual  : 1
              Test:           TestRunnerServiceServerExecute_WithInput/SimulateCtrlC

Almost fixed it. Still not quite done yet until https://github.com/stateful/runme/pull/607 is merged/release.

• Test_command/JavaScript and Test_command/JavaScriptEnv are failing due to different node.js version. We should run these only in the CI as locally it does not make sense. It kinda worked so far because all contributors have had the same node.js version. We can hide it with a build tag similarly to tests that expect Docker:

These should just migrate to https://github.com/stateful/runme/issues/605.