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

[orientdb] added test cases #569

Merged
merged 1 commit into from
Jan 4, 2016

Conversation

kruthar
Copy link
Collaborator

@kruthar kruthar commented Jan 3, 2016

Added basic operational test cases for the orientdb binding. Should give a little assurance to future cleanup work.

@kruthar
Copy link
Collaborator Author

kruthar commented Jan 4, 2016

So, this was a weird failure, seem that whatever location clojars was could not be reached.

I reran the build and things are green.

@@ -0,0 +1,228 @@
package com.yahoo.ycsb.db;
Copy link
Collaborator

Choose a reason for hiding this comment

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

File needs a license header.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed it does. Added.

@kruthar
Copy link
Collaborator Author

kruthar commented Jan 4, 2016

@busbey - thanks for the tip, exceptions now propogating

This is a copy of buildDeterministicValue() from core:com.yahoo.ycsb.workloads.CoreWorkload.java.
That method is neither public nor static so we need a copy.
*/
private String buildDeterministicValue(String key, String fieldkey) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any thoughts on this copying vs using something like powermock to access the method vs making the original visible for testing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thinking about this more, I agree that copying makes the most sense right now. It probably points to a need to come up with a common strategy for these kinds of tests, but that's not an issue for this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed on your last point, needs deeper thought on consolidated testing strategy in the future.

@busbey
Copy link
Collaborator

busbey commented Jan 4, 2016

+1

@kruthar
Copy link
Collaborator Author

kruthar commented Jan 4, 2016

Thanks @busbey

kruthar added a commit that referenced this pull request Jan 4, 2016
@kruthar kruthar merged commit 3ecf854 into brianfrankcooper:master Jan 4, 2016
@kruthar kruthar deleted the orientdb-testing branch January 4, 2016 14:55
@risdenk risdenk mentioned this pull request Feb 15, 2016
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
jaricftw pushed a commit to jaricftw/YCSB that referenced this pull request Jul 19, 2016
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