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

Be able to allocate a TTY when running tests #5897

Closed
thomasjm opened this issue Oct 8, 2022 · 6 comments
Closed

Be able to allocate a TTY when running tests #5897

thomasjm opened this issue Oct 8, 2022 · 6 comments

Comments

@thomasjm
Copy link

thomasjm commented Oct 8, 2022

I'm the author of the Sandwich test framework. One of Sandwich's key features is the ability to present a TUI interface for your tests. I think TUI-based testing is really useful, because it gives you a nice interface to watch and inspect the way your tests run.

The problem is, the default stack test behavior doesn't support this, because it doesn't allocate a TTY for the tests. I have a couple demos to demonstrate this:

git clone git@github.com:codedownio/sandwich.git

Normal operation: demo-basic is an executable

This is the workaround we've used until now--rather than using a test-suite, we just make the tests an executable and run them with stack run.

However, this has the downside that Stack builds all executables by default, so now library users have to build the tests even when they aren't planning on running them.

stack run demo-basic -- --tui

stack test operation: demo-stack-test is a test-suite

If you try to run a test-suite with the TUI, it errors out:

stack test demo-stack-test --test-arguments --tui
demo-stack-test> test (suite: demo-stack-test, args: --tui)

Some formatters failed: '[getTerminalAttributes: illegal operation (Inappropriate ioctl for device)]'
0 runs succeeded out of 1 repeat (3 tests)

Feature request

Thus, this is a feature request to modify how stack test works so it can use a TUI, similar to stack run. I'm not sure this should happen all the time, maybe it should be controlled by a flag?

I could take a stab at implementing this if I could get some guidance from a Stack dev.

@mpilgrem
Copy link
Member

mpilgrem commented Oct 8, 2022

@thomasjm, thanks for raising this.

It took me a little while to follow your examples: on Windows the --tui option is not offered; and on Ubuntu 22.04.1 LTS (on WSL) the dependency vty-5.33 initially failed to build with (extracts):

vty            > configure
vty            > Configuring vty-5.33...
vty            > build
vty            > Preprocessing library for vty-5.33..
vty            > Building library for vty-5.33..
vty            > [ 1 of 40] Compiling Codec.Binary.UTF8.Debug
...
vty            > [21 of 40] Compiling Graphics.Vty.Input.Loop
vty            >
vty            > <no location info>: warning: [-Wmissed-extra-shared-lib]
vty            >     libtinfo.so: cannot open shared object file: No such file or directory
vty            >     It's OK if you don't want to use symbols from it directly.
vty            >     (the package DLL is loaded by the system linker
vty            >      which manages dependencies by itself).
vty            > [22 of 40] Compiling Graphics.Vty.Input.Terminfo.ANSIVT
...
vty            > [40 of 40] Compiling Graphics.Vty.Inline
vty            > /usr/bin/ld.gold: error: cannot find -ltinfo
vty            > collect2: error: ld returned 1 exit status
vty            > `gcc' failed in phase `Linker'. (Exit code: 1)

I'd run into this before: #5771. It was fixed with:

apt install libtinfo-dev

So, I can see and reproduce the problem. Once built, demo-stack-test --tui can be run (from the demos/demo-stack-test directory) with:

.stack-work/dist/x86_64-linux-tinfo6/Cabal-3.4.1.0/build/demo-stack-test/demo-stack-test --tui

but when Stack runs the same executable with stack build --test, it falls over:

2022-10-08 20:06:21.106245: [debug] Run process within /home/mpilgrem/code/haskell/sandwich/demos/demo-stack-test/: /home/mpilgrem/code/haskell/sandwich/demos/demo-stack-test/.stack-work/dist/x86_64-linux-tinfo6/Cabal-3.4.1.0/build/demo-stack-test/demo-stack-test --tui
Some formatters failed: '[getTerminalAttributes: illegal operation (Inappropriate ioctl for device)]'
0 runs succeeded out of 1 repeat (3 tests)
2022-10-08 20:06:21.128531: [debug] Process finished in 22ms: /home/mpilgrem/code/haskell/sandwich/demos/demo-stack-test/.stack-work/dist/x86_64-linux-tinfo6/Cabal-3.4.1.0/build/demo-stack-test/demo-stack-test --tui

I'm going to see if I can find out where Stack runs the executable in its code.

@mpilgrem
Copy link
Member

mpilgrem commented Oct 8, 2022

Some notes for myself:

  • in the case of stack run, Main.execCmd makes use of RIO.Process.exec (which explains: Execute a process within the configured environment. Execution will not return ...)
    runWithPath eoCwd $ exec cmd args
  • in the case of a test, I think it is run in this part of Stack.Build.Execute.singleTest:
    let output =
            case outputType of
              OTConsole Nothing -> Nothing <$ inherit
              OTConsole (Just prefix) -> fmap
                (\src -> Just $ runConduit $ src .|
                  CT.decodeUtf8Lenient .|
                  CT.lines .|
                  CL.map stripCR .|
                  CL.mapM_ (\t -> logInfo $ prefix <> RIO.display t))
                createSource
              OTLogFile _ h -> Nothing <$ useHandleOpen h
        optionalTimeout action
          | Just maxSecs <- toMaximumTimeSeconds topts, maxSecs > 0 = do
              timeout (maxSecs * 1000000) action
          | otherwise = Just <$> action
    
    mec <- withWorkingDir (toFilePath pkgDir) $
      optionalTimeout $ proc (toFilePath exePath) args $ \pc0 -> do
        stdinBS <-
          if isTestTypeLib
            then do
              logPath <- buildLogPath package (Just stestName)
              ensureDir (parent logPath)
              pure $ BL.fromStrict
                $ encodeUtf8 $ fromString $
                show (logPath, mkUnqualComponentName (T.unpack testName))
            else pure mempty
        let pc = setStdin (byteStringInput stdinBS)
               $ setStdout output
               $ setStderr output
               pc0
        withProcessWait pc $ \p -> do
          case (getStdout p, getStderr p) of
            (Nothing, Nothing) -> pure ()
            (Just x, Just y) -> concurrently_ x y
            (x, y) -> assert False $ concurrently_ (fromMaybe (pure ()) x) (fromMaybe (pure ()) y)
          waitExitCode p

@thomasjm
Copy link
Author

Thanks for pointing me in the right direction @mpilgrem -- I was able to come up with a proof of concept!

https://github.com/codedownio/stack/tree/test-tty

If you install this version of stack, you can then do

stack test demo-stack-test --test-tty --test-arguments --tui

@mpilgrem
Copy link
Member

mpilgrem commented Oct 17, 2022

@thomasjm, thanks and interesting. It will take me a little while to work my way through it. Is the key difference in the let output =? Namely, the difference between (a) the Nothing <$ inherit of OTConsole Nothing or OTConsoleWithTTY and (b) the OTConsole (Just prefix) alternative?

If it is, I am wondering if these parts of Stack.Build.Execute.withSingleContext are relevant:

    -- Output to the console if this is the last task, and the user
    -- asked to build it specifically. When the action is a
    -- 'ConcurrencyDisallowed' action (benchmarks), then we can also be
    -- sure to have exclusive access to the console, so output is also
    -- sent to the console in this case.
    --
    -- See the discussion on #426 for thoughts on sending output to the
    -- console from concurrent tasks.
    console =
      (wanted &&
       all (\(ActionId ident _) -> ident == taskProvides) (Set.toList acRemaining) &&
       eeTotalWanted == 1
      ) || (acConcurrency == ConcurrencyDisallowed)

    withOutputType pkgDir package inner
        -- Not in interleaved mode. When building a single wanted package, dump
        -- to the console with no prefix.
        | console = inner $ OTConsole Nothing

It seems to me that at the heart of what --test-tty is doing is 'Output to the console also if (a) if it is a terminal (istty) and (b) the user has requested it'.

This is a link to the Stack issue mentioned in the comment: #426

@mpilgrem
Copy link
Member

@thomasjm, ignore my question above about the key difference being in the let output = - I think I now understand that it is not the key difference.

@thomasjm
Copy link
Author

thomasjm commented Oct 18, 2022

I'd say the key difference is invoking the test process with shell rather than proc here: https://github.com/codedownio/stack/blob/aa8966ed187266a06c402e100ad6110c0992ff47/src/Stack/Build/Execute.hs#L2016

Everything else is plumbing to support this. When you start the test process via a shell, it creates a pseudo-terminal which allows the TUI stuff to work.

A few thoughts about this solution:

  • The trick with invoking processes via shell is that you need to shell-escape the arguments to be totally correct. This is actually insanely involved and shell-specific, and normally I do this with the shell-escape package. Not sure how you feel about adding another dependency to Stack though.
  • Another solution, which can eliminate the need for shell stuff, is to use spawnWithPty from the posix-pty package. This approach would also be another dependency, although its own dependencies are minimal. Actually I just tried this and I don't think it'll work.
  • All of this is going to be POSIX-only btw, we'll have to gate things so none of this is compiled on Windows.

On a UX note, I made this opt-in via the --test-tty flag, but really the best ergonomics would be if an extra argument isn't required. I wonder if we could just make stack test always invoke the test with a TTY, when one is available? That seems nice and simple, I'm not sure what (if anything) it would break...

mpilgrem added a commit that referenced this issue Nov 19, 2022
The flag is enabled by default.
mpilgrem added a commit that referenced this issue Nov 19, 2022
Fix #5897 Add `--[no-]tests-allow-stdin` flag to `stack test`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants