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

e2e tests for hab pkg bulkupload, wired up into expeditor #7113

Merged
merged 5 commits into from Oct 31, 2019

Conversation

@jeremymv2
Copy link
Contributor

jeremymv2 commented Oct 25, 2019

Basic tests for hab pkg bulkupload

Caveat: This doesn't create a clean room Builder or origin. It merely tests for the proper exit code of commands. Perhaps a future iteration of these tests could verify against a clean room Builder, ensuring that it was properly mutated by the commands.

Signed-off-by: Jeremy J. Miller jm@chef.io

@jeremymv2 jeremymv2 requested review from christophermaier and markan Oct 25, 2019
@jeremymv2 jeremymv2 requested a review from chefsalim Oct 25, 2019
CMD=$2

echo
echo "==========Expected failure: Testing ${DESC}"

This comment has been minimized.

Copy link
@smacfarlane

smacfarlane Oct 25, 2019

Contributor

Not sure if it's appropriate on this particular line, but you can prefix your echo strings with --- to give it a nice fold in the buildkite UI. I've found this to be useful to break things up logically so that you can quickly find any output if you need to inspect the build logs

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 25, 2019

Author Contributor

Applied your suggestion in 4df23af

@jeremymv2 jeremymv2 force-pushed the jeremymv2/bulk_upload_e2e_tests branch from 4df23af to c0dc6c8 Oct 26, 2019
docker:
privileged: true
environment:
- ACCEPTANCE_HAB_AUTH_TOKEN

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

You're going to need a BUILD_PKG_TARGET in here.

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

Added in this commit e9f1516

echo "--- Testing with command ${HAB}, using cache dir ${CACHE_DIR}"
echo

before_upload() {

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

I thing these are fine, but it seems like we're very quickly approaching a point where we need to bring in a more formal framework of some kind. For instance, all this seems like it wants to be bats, and I've actually got some current work where getting a test environment for bats set up (with all the helper libraries) doesn't actually suck. aruba is another nice library in the Ruby world. And we've also discussed some kind of Powershell path to make cross-platform testing a bit easier.

I think these tests that you're writing are a bit different from other ones we've got, in that they're more about exercising CLI activity (others are about Supervisor functionality, which bring with them other constraints). That's a distinction that may be worth considering as we move forward.

This comment has been minimized.

Copy link
@christophermaier

christophermaier Oct 28, 2019

Contributor

Also, this is just an observation / food for thought... I'm not asking for a rewrite in bats (or anything else!) for this PR 😅

This comment has been minimized.

Copy link
@jeremymv2

jeremymv2 Oct 28, 2019

Author Contributor

I agree, I had added this bulkupload test to the end-to-end simply by following the precedent set from the hab pkg download e2e tests. So already, there's a pattern that isn't quite what we ideally want for CLI tests.

I'll bring up the CLI testing topic in the next Habitat planning session.

This comment has been minimized.

Copy link
@markan

markan Oct 29, 2019

Contributor

I'm +1 for merging this as is, but I think we should plan a spike near term to see if it's feasible to write a CLI test in powershell and make it work cross platform. If so, we should make that our standard going forward, and rewrite tests as a tech debt item.

@jeremymv2

This comment has been minimized.

Copy link
Contributor Author

jeremymv2 commented Oct 28, 2019

I think this issue is pertinent to tests like the ones in this PR
#7118

jeremymv2 added 4 commits Oct 25, 2019
Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
… keys

Signed-off-by: Jeremy J. Miller <jm@chef.io>
Signed-off-by: Jeremy J. Miller <jm@chef.io>
@jeremymv2 jeremymv2 force-pushed the jeremymv2/bulk_upload_e2e_tests branch from 875230c to 0305d64 Oct 28, 2019
Signed-off-by: Jeremy J. Miller <jm@chef.io>

export HAB_NOCOLORING=true
export HAB_NONINTERACTIVE=true
export HAB_ORIGIN="testbulkupload"

This comment has been minimized.

Copy link
@chefsalim

chefsalim Oct 29, 2019

Member

It would be great if we could use a more general test origin instead of specific to bulkupload.. I believe there has been some discussion already of a "habitat-test" origin

Copy link
Member

chefsalim left a comment

LGTM, just one comment re: the test origin

@markan
markan approved these changes Oct 30, 2019
@jeremymv2 jeremymv2 merged commit af02e6b into master Oct 31, 2019
5 checks passed
5 checks passed
DCO This commit has a DCO Signed-off-by
Details
buildkite/habitat-sh-habitat-master-verify Build #3967 passed (16 minutes, 7 seconds)
Details
buildkite/habitat-sh-habitat-master-website Build #1047 passed (1 minute, 33 seconds)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
expeditor/config-validation Validated your Expeditor config file
Details
@chef-expeditor chef-expeditor bot deleted the jeremymv2/bulk_upload_e2e_tests branch Oct 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.