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

separate redis test from the unit test #99

Open
jchauncey opened this issue Jul 7, 2016 · 7 comments
Open

separate redis test from the unit test #99

jchauncey opened this issue Jul 7, 2016 · 7 comments

Comments

@jchauncey
Copy link
Member

Since we are having to start an external component we are mixing terms when we call the test suit unit tests. Therefore, we should move the redis tests out to an integration target.

@arschles
Copy link
Member

arschles commented Jul 7, 2016

The separation is already done via the testredis build tag. The Makefile enables these tests in its test-unit target (https://github.com/deis/logger/blob/master/Makefile#L119), but the new build target is not in place

@jchauncey
Copy link
Member Author

Yeah but that tag says to run the redis tests. I am saying dont do that as part of test-unit. Instead do that as test-integration

@arschles
Copy link
Member

arschles commented Jul 7, 2016

Sorry, that tag is poorly named. It enables the redis tests, which run in addition to all the other tests. Thoughts on these test targets?

test-all:
  // runs all tests, including integration
  $(DOCKER_CMD) go build -tags="testredis" $(glide nv)
test-unit:
  // runs only unit tests, with no external dependencies
  $(DOCKER_CMD) go build $(glide nv)

@jchauncey
Copy link
Member Author

yeah. i would love to be able to just run the integration tests though.

@jchauncey
Copy link
Member Author

test-all: test-unit test-integration

Is what im thinking we should do

@arschles
Copy link
Member

arschles commented Jul 7, 2016

@jchauncey we could add capability to run just integration tests by checking a LOGGER_TEST_INTEGRATION env var in unit tests. If it exists, unit tests should bail out. I can research some other options too.

@Cryptophobia
Copy link

This issue was moved to teamhephy/logger#6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants