Use Docker instances of VoltDB for testing. #29
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Change the ddl to tear down the test Volt schema before setting it up. Change the run script to query Docker for the port Volt is exposed on.
nshi
suggested changes
Oct 2, 2017
test/util/dockerUtil.js
Outdated
| * which port the container's 21212 port has been exposed on. Intended to be used | ||
| * with the localhostcluster docker containers defined in the main volt repository. | ||
| */ | ||
| function _getExposedVoltPort(containerName) { |
There was a problem hiding this comment.
Maybe instead of throwing an error, this can return the default port number in case docker is not running or is not used for testing.
tools/testdb/run.sh
Outdated
| #$VOLTDB init -C deployment.xml -f -j $APPNAME.jar -s ddl.sql | ||
| #$VOLTDB start | ||
|
|
||
| PORT=`docker port node1 21212 | cut -d: -f2` |
There was a problem hiding this comment.
Similar to the comment made above, I think it'll be more friendly to fallback to starting a local VoltDB process if docker is either not running or not installed.
…after. As far as getting a Volt instance setup and ready for testing against: - It now defaults to starting a local Volt instance if it can't find a running Docker instance - If it does find a running docker instance it will use that On the testing side: - You can configure the preferred way to run the tests, but it defaults to using a local instance - You can override this by passing a cli option of -i docker to use docker. (Having the tests automagically search for a usable Volt instance gives me the heebie jeebies a bit... I know I switch between projects a lot and am a bit apprehensive about the tests running against a wrong instance). Hopefully it now supporting both ways (via switches) is a usable compromise? Some extra things... Have begun to add some linting Added nvm - minimum node version already quite out of date Some naming hygiene Run tests and lint from npm - still awkward but getting there
…length" error. Can only really guess what is triggering it given the sketchy details but the test case tries to replicate what is described in the VoltDB#18 issue. (VoltDB#28) Also contains some experimental promise based wrappers around the call back style fns in VoltClient... plus an eslint config.
|
Hi @nshi Should be OK to go now, take a look and let me know if you need some more changes. Messed up this merge a bit, was in a rush, hopefully I've tidied it up for you. |
nshi
approved these changes
Nov 16, 2017
…after. As far as getting a Volt instance setup and ready for testing against: - It now defaults to starting a local Volt instance if it can't find a running Docker instance - If it does find a running docker instance it will use that On the testing side: - You can configure the preferred way to run the tests, but it defaults to using a local instance - You can override this by passing a cli option of -i docker to use docker. (Having the tests automagically search for a usable Volt instance gives me the heebie jeebies a bit... I know I switch between projects a lot and am a bit apprehensive about the tests running against a wrong instance). Hopefully it now supporting both ways (via switches) is a usable compromise? Some extra things... Have begun to add some linting Added nvm - minimum node version already quite out of date Some naming hygiene Run tests and lint from npm - still awkward but getting there
Fix: Missing test context in buffer test Fix: Test for Volt docker instance in schema setup script Change: Return default Volt port when no Docker container found running volt
|
Found some time to go back and double check these changes. Needed a few fixes, which I've pushed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change the test cases to query Docker for the port Volt is exposed on.
Change the ddl to tear down the test Volt schema before setting it up.
Change the run script to query Docker for the port Volt is exposed on.
The objective is to:
Have tried to change existing code as little as possible for now, but will need a tidy up/refactor in future.