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

Beginnings of a test to track down the "Trying to read beyond buffer … #28

Merged
merged 1 commit into from
Sep 21, 2017

Conversation

mattyouill
Copy link
Contributor

…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 #18 issue.

Also contains some experimental promise based wrappers around the call back style fns in VoltClient... plus an eslint config.

…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.

Also contains some experimental promise based wrappers around the call back style fns in VoltClient... plus an eslint config.
@mattyouill
Copy link
Contributor Author

Hey this PR has some experimental API changes in it, wasn't sure how to raise these as design suggestions so have just put them in a test case rather than actually changing the client API. Otherwise it's just the beginnings of a test case to try and fix issue #18.

@nshi nshi merged commit c9479ea into VoltDB:master Sep 21, 2017
@nshi
Copy link
Contributor

nshi commented Sep 21, 2017

@mattyouil Thanks for the test. For the experimental API changes, if you feel like having a good handle on it, please feel free to write a short proposal of what you'd like to change and maybe break it down into smaller tasks. Then we can evaluate them.

@mattyouill mattyouill deleted the feature/buffer branch September 21, 2017 20:59
@mattyouill
Copy link
Contributor Author

mattyouill commented Nov 13, 2017

hi @nshi ... API thinking wasn't anything ground breaking, it's only I've noticed a lot of the API is callback based and there are some opportunities to perhaps bring it up to date with something more promise based - or perhaps observables?

A less dramatic way of introducing this might be to introduce these as "optional extensions", under a different namespace perhaps, that simply wrap the existing callback based API? An engineer can then choose to use the "base" API or the promise/observable based wrapper?

The test case submitted is an attempt to try out a promise based approach.

mattyouill added a commit to burnian-engineering/voltdb-client-nodejs that referenced this pull request Nov 14, 2017
…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.
nshi pushed a commit that referenced this pull request Dec 15, 2017
* 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.

* A few subtle changes in here but I think I've captured what you were 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

* Beginnings of a test to track down the "Trying to read beyond buffer 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 #18 issue. (#28)

Also contains some experimental promise based wrappers around the call back style fns in VoltClient... plus an eslint config.

* A few subtle changes in here but I think I've captured what you were 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: Order of statements in drop ddl
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants