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

Add support for running e2e tests locally, for rapid iteration #6855

Merged
merged 3 commits into from Aug 16, 2019

Conversation

@christophermaier
Copy link
Contributor

commented Aug 15, 2019

Adds an e2e_local.sh script that can be used to run an individual
end-to-end test script in an isolated Docker environment that mimics
what we use in CI.

Documentation is included, but the TL;DR is

./e2e_local.sh test/end-to-end/${YOUR_TEST_FILE}

Signed-off-by: Christopher Maier cmaier@chef.io

Adds an `e2e_local.sh` script that can be used to run an individual
end-to-end test script in an isolated Docker environment that mimics
what we use in CI.

Documentation is included, but the TL;DR is

```sh
./e2e_local.sh test/end-to-end/${YOUR_TEST_FILE}
```

Signed-off-by: Christopher Maier <cmaier@chef.io>
@chef-expeditor

This comment has been minimized.

Copy link

commented Aug 15, 2019

Hello christophermaier! Thanks for the pull request!

Here is what will happen next:

  1. Your PR will be reviewed by the maintainers.
  2. If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

Copy link
Contributor

left a comment

This is really awesome!

I made some minor suggestions, but nothing mandatory since it's super useful as is. Additionally, would it make sense to move most of this to the test/end-to-end directory?

One other suggestion that would be extra awesome is making this script parse the end_to_end.pipeline.yml so that it could be even more authentic to the way CI runs. But that's probably a follow-on task along with converting the bash to rust to make it runnable on windows.

raw_commands=(". /opt/ci-studio-common/buildkite-agent-hooks/ci-studio-common.sh"
".expeditor/scripts/setup_environment.sh DEV"
"hab pkg install --binlink --channel=stable core/expect"
"${*}")

This comment has been minimized.

Copy link
@baumanj

baumanj Aug 15, 2019

Contributor

Should this be

Suggested change
"${*}")
"${@}")

As it is, it limits the input to passing only one command to the script. Changing this allows each argument to be a separate command; the callers just need to make each one a separate argument:

$ e2e_local.sh "cmd1 arg2 arg2" "cmd2 arg1 arg2"

This comment has been minimized.

Copy link
@christophermaier

christophermaier Aug 15, 2019

Author Contributor

I was trying to keep it as close to CI as possible, while also encouraging us to move toward things being as self-contained and consistent as possible. Ideally, I think I'd like each test to just be a single executable file.

To that end, this actually started as ${1}, but some test files currently take arguments.

"${*}")

# Add a newline after every command, for feeding into the container.
commands="${raw_commands[*]/%/\\n}"

This comment has been minimized.

Copy link
@baumanj

baumanj Aug 15, 2019

Contributor

I think this should use [@] rather than [*]. Additionally, would it be simpler to append ; to separate commands? I think that would eliminate the need for ${commands@E} later. Eliminating that dependency would be nice, since it's a feature that doesn't exist in the version of bash that ships with macOS.

This comment has been minimized.

Copy link
@christophermaier

christophermaier Aug 15, 2019

Author Contributor

Looks like ; works fine, too. The \n approach was in keeping with what Buildkite actually executes, but it's functionally equivalent and (as you observe) a bit more cross-platform friendly.

I'll make the change.

This comment has been minimized.

Copy link
@christophermaier

christophermaier Aug 15, 2019

Author Contributor

I looked at * vs @ and it looks like it really does have to be *;

 ${commands[@]/%/;}  # -> first 'command;' second 'command;' third 'command;'
 ${commands[*]/%/;}  # -> first 'command;' second 'command;' third 'command;'
"${commands[@]/%/;}" # -> 'first command;' 'second command;' 'third command;'
"${commands[*]/%/;}" # -> 'first command; second command; third command;'

Only the last one gives us what we want; a single string of delimited commands to pass into bash -c.

This comment has been minimized.

Copy link
@baumanj

baumanj Aug 15, 2019

Contributor

If you only want to allow one command (with arguments) as input to e2e_local.sh and not have to put quotes around them; I agree. Using @ is necessary if you want to allow more than one command on the CLI, but it would have to be invoked like e2e_local.sh "cmd1 arg1" "cmd2 arg2"

This comment has been minimized.

Copy link
@christophermaier

christophermaier Aug 16, 2019

Author Contributor

I think for this iteration, I'll leave the code as-is, since I think it'll be a good forcing function to move us toward more standardization.

If that's trickier to do in practice, then we can easily modify things as laid out in these comments.

e2e_local.sh Show resolved Hide resolved
Signed-off-by: Christopher Maier <cmaier@chef.io>
Signed-off-by: Christopher Maier <cmaier@chef.io>
@christophermaier christophermaier merged commit 998c7bf into master Aug 16, 2019
5 checks passed
5 checks passed
DCO This commit has a DCO Signed-off-by
Details
buildkite/habitat-sh-habitat-master-verify Build #3124 passed (20 minutes, 47 seconds)
Details
buildkite/habitat-sh-habitat-master-website Build #252 passed (34 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details
@chef-ci chef-ci deleted the cm/run-e2e-locally branch Aug 16, 2019
@baumanj

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

@christophermaier

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

@baumanj it could... we'd need to juggle some pathing internally to get the right docker mounts, but that's just typing 😂

@baumanj

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2019

Weigh that against whether people who work on the e2e tests are more likely to discover this stuff if it's more local to the directory, I guess. I'm fine with either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.