spring-projects / spring-shell

Spring based shell
http://projects.spring.io/spring-shell/
Apache License 2.0
732 stars 396 forks source link

InteractiveShellSession refuse to complete #924

Open dogruis opened 11 months ago

dogruis commented 11 months ago

When ending a session I use this code after each test because one simple ctrl + c doesn't guarantee that the session will be completed. I run my tests in Jenkins and there is around 100 tests. This was flaky until the following method was added.

If the tests are run without forcing the session to complete, the last inputs from the previous test appear in the next test first input. Note: this was occuring randomly.

    @AfterEach
    @SneakyThrows
    void after() {
        var session = client.interactive().run();
        await().pollDelay(0, SECONDS)
                .pollInterval(1, SECONDS)
                .atMost(30, SECONDS)
                .untilAsserted(() -> {
                    session.write(session.writeSequence().ctrl('c').build());
                    assertThat(session.isComplete()).isTrue();
                });

        terminal.writer().flush();
        terminal.close();
        client.close();
    }
jvalkeal commented 11 months ago

Right, I think I've seen something similar in ShellTestIntegrationTests which have been difficult to track down as something only randomly fail on CI. I'll play with your snippet if that gives more reliable way to find the issue.

ascopes commented 11 months ago

Wondering if this is related to the session and the underlying terminal being shared between tests, even with DirtiesContext in place. I'd assume this shouldn't be happening ideally.

dogruis commented 11 months ago

I thought this would be related to the way the interactive shell session is working. It is running a thread and seeing the random nature of the bug this could be related. The ShellTestClient is using this:

        @Override
        public InteractiveShellSession interactive() {
            terminalSession.start();
            if (runnerThread == null) {
                runnerThread = new Thread(new ShellRunnerTask(this.blockingQueue));
                runnerThread.start();
            }
            return new DefaultInteractiveShellSession(shell, promptProvider, lineReader, blockingQueue, terminalSession, terminal);
        }
dogruis commented 11 months ago

Right, I think I've seen something similar in ShellTestIntegrationTests which have been difficult to track down as something only randomly fail on CI. I'll play with your snippet if that gives more reliable way to find the issue.

This code snippet was written in order to mittigate the issue.

dogruis commented 11 months ago

I fixed the issue by stopping maven from running with the -T2C argument and I added the @Isolated tag to each class running these spring-shell integration tests. It is now running safely but the test are forced to run in isolation which makes the build slower. The issue in spring-shell-test library is still there!