fix: tests now work with different sh implementations

[finnag]

what

Ignore output from commands that test failure scenarios with sh in workflow hooks

why

4915d8d introduced output checking of failing commands in workflow hook script tests. However, the output from sh is not stable between different platforms, so this broke tests on at least Mac OS X.

The following commands have unstable output between architectures:

sh -c "echo 'a"
sh -c "jlkjlkefjelkfjelkfj"

Fix by interpreting empty expected output as any output is valid in this test.

tests

I have tested my changes by

• make docker/test
• make test # on a mac

references

• fix(workflow-hook): log output with or without errors #3422

[nitrocode]

cc @X-Guardian

[nitrocode]

Thank you @finnag for contributing this. Are you running atlantis on macos?

Since you, @X-Guardian , contributed #3422, could you be a second pair of eyes and review this PR to ensure this works and is implemented as expected?

[X-Guardian]

@finnag, please detail the exact output you saw on the mac. It would be better to adjust the check to cope with the various output differences rather than ignore the check altogether.

[finnag]

I sometimes develop on a mac, and run atlantis locally while developing. That's where I usually run the tests. In production it runs on a linux server somewhere. Looks like mac outputs the same as centos.

There are many sh variants, here are some:

% /bin/sh -c "echo 'a"   # locally on a Mac
/bin/sh: -c: line 0: unexpected EOF while looking for matching `''
/bin/sh: -c: line 1: syntax error: unexpected end of file

% docker run centos /bin/sh -c "echo 'a"
/bin/sh: -c: line 0: unexpected EOF while looking for matching `''
/bin/sh: -c: line 1: syntax error: unexpected end of file

% docker run alpine /bin/sh -c "echo 'a" 
/bin/sh: syntax error: unterminated quoted string

% docker run debian /bin/sh -c "echo 'a" 
/bin/sh: 1: Syntax error: Unterminated quoted string

and for the unknown command:

% /bin/sh -c jefjef
/bin/sh: jefjef: command not found

% docker run centos /bin/sh -c jefjef
/bin/sh: jefjef: command not found

% docker run alpine /bin/sh -c jefjef 
/bin/sh: jefjef: not found

% docker run debian /bin/sh -c jefjef 
/bin/sh: 1: jefjef: not found

[finnag]

@X-Guardian what do you think? the current tests require dash as /bin/sh. fails with bash (macos, centos) and busybox/ash (alpine). Doesn't feel necessary to force developers to use dash or docker to run the tests.

[GenPage]

I agree with @finnag here, we shouldn't be checking for shell outputs in unit tests. If we really feel it's vital to be checking shell outputs as part of testing, it should be moved to e2e tests where proper testing against different shell programs can be setup and performed.

[finnag]

updated to match latest main.

[github-actions]

This issue is stale because it has been open for 1 month with no activity. Remove stale label or comment or this will be closed in 1 month.

[finnag]

There is now an alternative fix for this in main, so this is no longer required.