ci: make failing tests fail the CI

[florentsfl]

Currently, when a test is failing, the CI does not fail because the script does not detect correctly a failure.

This commit makes the test of a failing test more explicit

[florentsfl]

But there is an issue with

**|FAIL|cqfd without redirection should allocate a tty

@gportay do you have an idea if it can be solved in GitHub Actions ?

[gportay]

@florentsfl, tty in the container outputs not a tty.

[18:30:10|notice] preparing "cqfd without redirection should allocate a tty"
not a tty
[18:30:11|info] result: cqfd without redirection should allocate a tty: FAIL

Since it outputs something, I strongly think docker has allocated a pts device. Therefore I think the issue is in how tty find out the device path.

I guess tty reads the /proc filesystem (probably /proc/self/fd/0). Does the procfs is mounted in the GitHub action and in the container?

[gportay]

It needs /dev to be mounted has well, with /dev/stdin as a symlink to /proc/self/fd/0. I would starts investigating from if these peaces are in the system and in the container.

[gportay]

strace says it readlink("/proc/self/fd/0", "/dev/pts/4", 4095) = 10.

gportay@archlinux ~/src/cqfd $ strace tty 
(...)
ioctl(0, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
ioctl(0, TCGETS, {c_iflag=ICRNL|IXON|IUTF8, c_oflag=NL0|CR0|TAB0|BS0|VT0|FF0|OPOST|ONLCR, c_cflag=B38400|CS8|CREAD, c_lflag=ISIG|ICANON|ECHO|ECHOE|ECHOK|IEXTEN|ECHOCTL|ECHOKE, ...}) = 0
fstat(0, {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0x4), ...}) = 0
readlink("/proc/self/fd/0", "/dev/pts/4", 4095) = 10
newfstatat(AT_FDCWD, "/dev/pts/4", {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0x4), ...}, 0) = 0
fstat(1, {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0x4), ...}) = 0
write(1, "/dev/pts/4\n", 11/dev/pts/4
)            = 11
close(1)                                = 0
close(2)                                = 0
exit_group(0)                           = ?
+++ exited with 0 +++

[florentsfl]

actions/runner#241 and https://github.com/gfx/example-github-actions-with-tty i’m gonna investigate that

[florentsfl]

So there is this hacky solution to allocate a TTY using the script command as a shell, or we can just skip the failing test in GitHub Actions so we don’t merge code that may break something later. What do you think @gportay ?

[gportay]

So there is this hacky solution to allocate a TTY using the script command as a shell, or we can just skip the failing test in GitHub Actions so we don’t merge code that may break something later. What do you think @gportay ?

I have no strong opinion about that script. If it works, go with it.

I do not mind skipping the tests as well.

jtest_prepare "cqfd without redirection should allocate a tty"
if tty; then
  if $cqfd | grep "/dev/pts/1" ; then
  	jtest_result pass
  else
	  jtest_result fail
  fi
else
  jtest_result skip
fi

I have not tested what jtest_result skip do.

OOS, Maybe we should do that kind of change for that test.

[florentsfl]

jtest_result skip does not exist but I’m gonna implement that because it seems useful

[gportay]

I suggest to test for NO_COLOR and to test if stdout is tty.

[florentsfl]

I added NO_COLOR and i tried [ -t 1 ] but it seems to fail in GH Actions

[gportay]

It is definitely not a tty:

I strace tty from GitHub action, and I get:

ioctl(0, TCGETS, 0x7ffc276301d0)        = -1 ENOTTY (Inappropriate ioctl for device)

So I was wrong with the /dev and /proc filesystems. and you were right with the script.

[gportay]

They are pipes if I am correct:

+ test -t 0
+ echo 0 is a not tty
+ test -t 1
+ echo 1 is a not tty
+ test -t 2
+ echo 2 is a not tty
+ test -r /dev/stdin
+ echo /dev/stdin is readable
+ test -r /dev/stdout
+ echo /dev/stdout is readable
+ test -r /dev/stderr
+ echo /dev/stderr is readable
+ readlink /proc/self/fd/0
0 is a not tty
1 is a not tty
2 is a not tty
/dev/stdin is readable
/dev/stdout is readable
/dev/stderr is readable
pipe:[10230]
+ echo /proc/self/fd/0 is a symlink
/proc/self/fd/0 is a symlink
+ readlink /proc/self/fd/1
pipe:[10231]
+ echo /proc/self/fd/1 is a symlink
+ readlink /proc/self/fd/2
/proc/self/fd/1 is a symlink
pipe:[10232]
+ echo /proc/self/fd/2 is a symlink
/proc/self/fd/2 is a symlink

[gportay]

i tried [ -t 1 ] but it seems to fail in GH Actions

By failing, I guess you mean you have no color if adding the test [ -t 1]?

[florentsfl]

yes that was returning non colored text when adding this test

[gportay]

That was normal since it is not a tty.

[florentsfl]

Ok but GH Actions is able to render colors even not being a tty, so I thought the test was too restrictive

[gportay]

Indeed.

[florentsfl]

Thanks for the comments ! It should be good now

[deribaucourt]

LGTM, maybe we should consider using a proper shell testing framework rather than defining coloration ourselves?

[florentsfl]

Yes, I took the colorize function from cukinia, but if you have an example of a framework we can use, why not !

[gportay]

LGTM, for what it worth.

[deribaucourt]

@florentsfl We could use cukinia itself, or bats to run the tests in the future.
This PR will do, I suggest to merge this one as it is.