Docker - the input device is not a TTY

[andrepm06]

I'm trying to run my tests on a TFS build. However, I got the following error:

the input device is not a TTY

I know that this is related to -it flag on docker run command, but I'm afraid that i cannot remove this flag without submitting a PR (I dont know why backstop uses -it also) and I cannot activate tty on the server in which TFS is running.

Any ideas about that?

[garris]

Unfortunately no -- I don't know what the issue is there. Perhaps there is more info in docker documentation.

[andrepm06]

I'll dig out the documentation.

Is there a particular reason for using -it ondocker run ?

[garris]

Is there a particular reason for using -it ondocker run ?

Hmm, I don't remember what the reason was -- we have been through some different versions. Docker is kind of great -- but it is also kind of a pain. Different pipelines have different requirements. One size definitely does not fit all cases.

[andrepm06]

Yes, definitely.

I actually was thinking in trying something like this:

"dockerOptions": { "args": ["-it"] }

[garris]

This is a very tricky thing to do since there is an existing default set of arguments. What would be your proposal for managing this? Another approach to consider is a documented template approach similar to what we did here... https://github.com/garris/BackstopJS#if-you-just-upgraded-to-2x-or-3x

I am open to suggestions.

[gyanta]

I have the same issue.
According to the Docker docs:

Specifying -t is forbidden when the client standard output is redirected or piped, such as in: echo test | docker run -i busybox cat.

I sent a PR earlier to make docker run more customizable: #925
I fixed this particular issue by adding "-t=false" to dockerRunExtraArgs. (which is the same as using -i instead of -it)
-t seems to be necessary only if we want to handle keyboard input while running in the container. Is there a use case for that?

If anyone comes up with a cleaner design, I'm more than happy to put together another pull request.

[ephjos]

I have the same issue.
According to the Docker docs:

Specifying -t is forbidden when the client standard output is redirected or piped, such as in: echo test | docker run -i busybox cat.

I sent a PR earlier to make docker run more customizable: #925
I fixed this particular issue by adding "-t=false" to dockerRunExtraArgs. (which is the same as using -i instead of -it)
-t seems to be necessary only if we want to handle keyboard input while running in the container. Is there a use case for that?

If anyone comes up with a cleaner design, I'm more than happy to put together another pull request.

The solution in #925 worked for me. Working both locally and on our build machines.

It seems as though the only change after passing "-t=false" is that the output of puppeteer is no longer colorized.

Before:

After:

[screendriver]

Any news on that? How did you get this working @josephthomashines? The PR #925 is still open.

[ephjos]

@screendriver I just installed from the fork made for #925 , and passed the option in the config.

package.json

backstop config

[screendriver]

Thanks. That was the same thing that I had in mind but I thought there would be a "nicer" solution for that.

@garris what about merging #925 ?

[garris]

@screendriver @josephthomashines @gyanta @andrepm06
Hi guys, Very sorry for the delay. I have been a little bit stuck on this one because I think it may be a bit of a whack-a-mole problem. Solving for extra args and multiple versions may not cover all needs for all environments. I think we want to avoid config sprawl. I think we need something more flexible.

I very much appreciate @gyanta 's PR. But before going forward with that I want to propose a different option...

Contex: A similar issue happened with output filenames. There were a few requests asking to modify this-or-that part of the filename -- and proposals for multiple parameters were coming in. To solve this we simply added a fileNameTemplate -- which could be used to give output filenames a completely custom/arbitrary structure but would also allow users to apply runtime parameters where needed -- described in short here... https://github.com/garris/BackstopJS#if-you-just-upgraded-to-2x-or-3x

I would like to propose doing this with the docker command. This way as new docker features/formats or user/environment needs arize, devs will be free to change this command in any arbitrary way. One could even run some other wrapper or other container system or even some arbitrary shell command for that matter.

Anyway, more specifically I'd propose a parameter like...

dockerCommandTemplate: 'docker run --rm -it --mount type=bind,source="{cwd}",target=/src backstopjs/backstopjs:{version} {backstopCommand} "{args}"';

where internally replacements are done against these runtime variables...

// cwd = process.cwd()  [the current working directory]
// version = Runtime backstopjs version.
// backstopCommand = The command passed to backstopjs e.g. test, reference etc.
// args = Args passed when calling BackstopJS from node plus any args passed via the command line.

So something simple like changing parameters would look like...

dockerCommandTemplate: 'docker run --rm -i -t=false --mount type=bind,source="{cwd}",target=/src backstopjs/backstopjs:{version} {backstopCommand} "{args}"';

But you could also do something radical like...

dockerCommandTemplate: 'newCoolContainerSystem run -someParam --mount type=bind,source="{cwd}",target=/src someOtherUser\'s/backstopImage {backstopCommand} "{args}"';

So that's one fairly future-proof API change. Thoughts?

[ephjos]

No worries about the delay, I think that this approach is a great idea! It would allow for small tweaks, like the one I need to make, but also could open doors to more complex setups.

[gyanta]

I agree @garris, this is the right way to do it, I'd prefer this over my PR.

[garris]

Thanks guys, I appreciate the consideration! And I appreciate all the patience too -- I wish I could be better about following up quicker. So busy 😅

[screendriver]

I'm fine with that as well ☺️ Additionally I guess this is a really small change in your codebase, right?

[garris]

Great -- thanks for the follow up. And yes, should be a bit less code -- which is a good thing. 🙂

[gyanta]

@garris @screendriver @josephthomashines @andrepm06 Not sure if anyone else is already working on this - would you mid having a look at #1013?