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

[Updated] Add quick and full integration test targets #355

Closed
wants to merge 2 commits into from

Conversation

sehqlr
Copy link
Contributor

@sehqlr sehqlr commented Oct 6, 2016

This commit adds integration test code in two phases:

  • quick-integration.hcl, for CI builds. Run with make integration
  • full-integration.sh, for nightly builds. This is triggered by setting
    CVG_FULL_INTEGRATION in the environment

We are using docker for testing the same HCL file
against multiple linux OSes. We need vagrant because we need
an environment to test LVM modules, and it's a test of our plugin.

Future Work:

  • Port blackbox tests to integration tests where appropriate
  • Set up nightly build machine, and add integration test to wercker
  • Right now, the docker target creates containers serially. I would like
    to add a target where we simulate a cluster with containers. Should we
    use docker compose for that? an HCL file? what?

This commit adds integration test code in two phases:

- quick-integration.hcl, for CI builds. Run with `make integration`
- full-integration.sh, for nightly builds. This is triggered by setting
  CVG_FULL_INTEGRATION in the environment

We are using docker for testing the same HCL file
against multiple linux OSes. We need vagrant because we need
an environment to test LVM modules, and it's a test of our plugin.

Future Work:

- Port blackbox tests to integration tests where appropriate
- Set up nightly build machine, and add integration test to wercker
- Right now, the docker target creates containers serially. I would like
  to add a target where we simulate a cluster with containers. Should we
  use docker compose for that? an HCL file? what?
Copy link
Contributor

@ryane ryane left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! the quick integration tests worked great for me. we can clean up some of the tasks as we introduce more features (such as #253) but this seems like a great start.

I left some other feedback around the full integration test which does not really work right now. What does everyone think about just moving forward with the quick tests so that we at least have some integration testing in place while we work on solving the remaining problems?

@@ -0,0 +1,5 @@
#!/bin/bash
cd $(dirname "$0")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

untested but maybe try something like cd "$(dirname "$0")" || exit 1 to cleanup code climate issues

@echo
@echo === quick integration test ===
$(INTEGRATION_CMD) testing/quick-integration.hcl
@ if [ -n "$$CVG_FULL_INTEGRATION" ];\
Copy link
Contributor

@ryane ryane Oct 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably document the use of this env var somewhere. maybe in CONTRIBUTING.md?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add a note there, good catch

]
cvg.install = true
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is out of date with the most recent version of the Vagrant plugin.

There are errors in the configuration of this machine. Please fix
the following errors and try again:

vagrant-converge:
* The following settings shouldn't exist: bikeshed
* No value provided for hcl


There are errors in the configuration of this machine. Please fix
the following errors and try again:

vagrant-converge:
* The following settings shouldn't exist: bikeshed
* No value provided for hcl

Also, effective testing using vagrant is blocked by #356. I wonder if we should just leave this part out of the PR until we solve those problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I'll remove it. I'll switch it back to echo NOT IMPLEMENTED YET, and that'll fix the code climate issue too

Since vagrant is currently blocked, we will need to add it back in at a
later time. I also added in a new testing section to CONTRIBUTING that
describes our various testing make targets
@BrianHicks BrianHicks removed this from the 0.3.0 milestone Oct 13, 2016
@BrianHicks
Copy link
Contributor

Thanks for this, Sam. We'll probably use this as a basis for integration testing in the future but I'm going to close it for now so we have a clearer view of our open PRs.

@BrianHicks BrianHicks closed this Dec 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants