buildkite-plugins / buildkite-plugin-tester

🔨 A base Docker image for testing Buildkite plugins
MIT License
11 stars 10 forks source link

bats-mock: `*` pattern no longer works #51

Closed JAORMX closed 2 years ago

JAORMX commented 2 years ago

Issue

the stub command from the bats-mock library is normally able to take the * to denote that the stub will handle any arguments for the given command/binary that's being stubbed. This functionality is no longer usable, as the * now globs files present in the working directory.

Example invocation:

stub buildkite-agent "* : exit 0"

Example output:

 bats-mock(buildkite-agent): got buildkite-agent annotate --style error failed to run uname<br /> --context trivy-scan                                                                                                                                                                                                  16/30
bats-mock(buildkite-agent): patterns  [7] = 'LICENSE' 'Makefile' 'README.md' 'hooks' 'plugin.yml' 'renovate.json' 'tests'
bats-mock(buildkite-agent): arguments [6] = 'annotate' '--style' 'error' 'failed\ to\ run\ uname\<br\ /\>' '--context' 'trivy-scan'
bats-mock(buildkite-agent): match failed at idx 0, expected 'LICENSE', got 'annotate'
bats-mock(buildkite-agent): result 1
bats-mock(buildkite-agent): unstubbing

Expected behavior

Using the following format should stub the command with any given number of parameters:

stub <command> "* : <stub command>"

Broken since

buildkite plugin-tester v3.0.0

Note that it's still broken in v3.0.1

Worked in

buildkite plugin-tester v2.0.0

toote commented 2 years ago

Due to the big change in how stub definitions are parsed this also changed. You need to escape the * in the stub as follows:

stub <command> "\* : <stub command>"

That is the way currently being used in other plugins that have already been updated to the latest version (like docker compose

JAORMX commented 2 years ago

@toote I actually tried to escape it but that didn't work. The pattern that was captured was \* and that didn't capture the stub call anymore.

toote commented 2 years ago

That is weird, running the tests I linked from docker compose shows it working:

bats-mock(docker): got docker ps -a --filter label=com.docker.compose.project=buildkite1111 --format {{.ID}}\t{{.Label "com.docker.compose.service"}}
bats-mock(docker): patterns  [6] = 'ps' '-a' '--filter' 'label=com.docker.compose.project=buildkite1111' '--format' '\*' 
bats-mock(docker): arguments [6] = 'ps' '-a' '--filter' 'label=com.docker.compose.project=buildkite1111' '--format' '\{\{.ID\}\}\\t\{\{.Label\ \"com.docker.compose.service\"\}\}' 
bats-mock(docker): running cat tests/fixtures/id-service-multiple-services.txt
bats-mock(docker): command input is 
bats-mock(docker): command result was 0
bats-mock(docker): result 0
JAORMX commented 2 years ago

I'll give it another try tomorrow. I'll get back to you on this one.

JAORMX commented 2 years ago

Alright. I gave this another try. Without the character escape and with debugging enabled, I get the following error:

 bats-mock(curl): got curl --fail -L https://github.com/aquasecurity/trivy/releases/download/v6.6.6/trivy_6.6.6_checksums.txt                                                                                                                                                                                           20/31
bats-mock(curl): patterns  [7] = 'LICENSE' 'Makefile' 'README.md' 'hooks' 'plugin.yml' 'renovate.json' 'tests'
bats-mock(curl): arguments [3] = '--fail' '-L' 'https://github.com/aquasecurity/trivy/releases/download/v6.6.6/trivy_6.6.6_checksums.txt'
bats-mock(curl): match failed at idx 0, expected 'LICENSE', got '--fail'
bats-mock(curl): result 1
bats-mock(buildkite-agent): got buildkite-agent annotate --style error download_trivy: downloaded hashes file is empty<br /> --context trivy-scan
bats-mock(buildkite-agent): patterns  [7] = 'LICENSE' 'Makefile' 'README.md' 'hooks' 'plugin.yml' 'renovate.json' 'tests'
bats-mock(buildkite-agent): arguments [6] = 'annotate' '--style' 'error' 'download_trivy:\ downloaded\ hashes\ file\ is\ empty\<br\ /\>' '--context' 'trivy-scan'
bats-mock(buildkite-agent): match failed at idx 0, expected 'LICENSE', got 'annotate'
bats-mock(buildkite-agent): result 1
bats-mock(buildkite-agent): unstubbing
 ✗ download_trivy: curl hashes file failure
   (in test file tests/pre-checkout.bats, line 61)
     `[ "$status" -eq 41 ]' failed
   BUILDKITE_AGENT_STUB_DEBUG

Note that this is just an error for one test, the whole test suite that used the pattern is now failing.

As we know. The * gets "globbed" and we see it expands to the files in the directory. as seen in the following log line:

bats-mock(buildkite-agent): patterns  [7] = 'LICENSE' 'Makefile' 'README.md' 'hooks' 'plugin.yml' 'renovate.json' 'tests'

Now, I created the PR https://github.com/equinixmetal-buildkite/trivy-buildkite-plugin/pull/42 which escapes the * character. I get the following error:

 bats-mock(curl): got curl --fail -L https://github.com/aquasecurity/trivy/releases/download/v6.6.6/trivy_6.6.6_checksums.txt                                                                                                                                                                                           20/31
bats-mock(curl): patterns  [1] = '\*'
bats-mock(curl): arguments [3] = '--fail' '-L' 'https://github.com/aquasecurity/trivy/releases/download/v6.6.6/trivy_6.6.6_checksums.txt'
bats-mock(curl): unexpected argument '-L' at idx 1
bats-mock(curl): result 0
bats-mock(buildkite-agent): got buildkite-agent annotate --style error download_trivy: downloaded hashes file is empty<br /> --context trivy-scan
bats-mock(buildkite-agent): patterns  [1] = '\*'
bats-mock(buildkite-agent): arguments [6] = 'annotate' '--style' 'error' 'download_trivy:\ downloaded\ hashes\ file\ is\ empty\<br\ /\>' '--context' 'trivy-scan'
bats-mock(buildkite-agent): unexpected argument '--style' at idx 1
bats-mock(buildkite-agent): result 0
bats-mock(buildkite-agent): unstubbing
 ✗ download_trivy: curl hashes file failure
   (in test file tests/pre-checkout.bats, line 61)
     `[ "$status" -eq 41 ]' failed
   BUILDKITE_AGENT_STUB_DEBUG

Note that patterns is now detected directly as \*. The interesting thing is that it seems to match something... but only one argument and not all as previously expected.

JAORMX commented 2 years ago

So... There seems to be a change of behavior in the * character. We can work with that, but it would be nice to have an alternative.

toote commented 2 years ago

I hadn't realized that scenario as it is definitely a change in behaviour... but I'm not sure that the previous behaviour was the correct one either. There has been a PR for years in bats-mock adding tests that would specifically indicate that * should match a single parameter and not everything else.

JAORMX commented 2 years ago

@toote got it. Well, it would be good to add docs that clarify this behavior. I'll close this issue then. Thanks for the responsiveness!