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.

Sign up for GitHub

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

Jump to bottom

acceptance: need to refactor the Java acceptance tests #4591

Closed
knz opened this issue Feb 23, 2016 · 9 comments
Closed

acceptance: need to refactor the Java acceptance tests #4591

knz opened this issue Feb 23, 2016 · 9 comments
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience
Milestone
Later

Comments

@knz
Copy link
Contributor

knz commented Feb 23, 2016

Suggestions collected over #4578:

  • Java code should be in separate .java files (eases editing)
  • there should be no preprocessing (different files are best)
  • the different Java tests should be executed one after another by test logic written in Java
  • the filesystem in the test docker instance should not be polluted by generated files
@knz knz mentioned this issue Feb 23, 2016
Closed
@knz knz added this to the 1.0 milestone Feb 23, 2016
@knz knz added C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience labels Feb 23, 2016
@petermattis petermattis changed the title Need to refactor the Java acceptance tests acceptance: need to refactor the Java acceptance tests Mar 31, 2016
@a6802739
Copy link
Contributor

@knz, you mean we should write separate .java files instead of java_test.go

@knz
Copy link
Contributor Author

knz commented Jul 18, 2016

@a6802739 yes that is correct.

@a6802739
Copy link
Contributor

a6802739 commented Jul 19, 2016 •
edited
Loading

@knz , thanks, another question, where is the code to control all the execute logic in acceptance. is it run.sh?

@knz
Copy link
Contributor Author

knz commented Jul 19, 2016

Yes, acceptance/run.sh calls go test which in turn runs all the Go tests defined in the directory.

You can also run make acceptance from the top-level directory, which will do the same thing.

@a6802739
Copy link
Contributor

@knz , if I change java_test.go to separate .java files. I should also change run.sh and makefile, right?

@knz
Copy link
Contributor Author

knz commented Jul 20, 2016

Yes you can approach it this way.

Meanwhile to align with our other tests I could also suggest 1 Go acceptance test function which:

  1. starts the test DB server
  2. invokes the Java compiler to compile the Javatest code (and checks there are no compile errors)
  3. with a loop runs each test Java program, giving it the DB connection string as argument each time
  4. when all Java tests have run, stops the test DB server

Of course if you have a better idea please go forward with it.

@a6802739
Copy link
Contributor

@knz, thanks for you advice.

@rohangulati
Copy link

@knz, I would like work on this issue. I have on question though. How are we supposed to copy the .java files from our host to the docker container after it is started?
Presently, we are creating these .java files on the container by executing a bash script on the container itself. Right?

@knz
Copy link
Contributor Author

knz commented Nov 7, 2016

Hi @rohangulati! Thanks for offering to help!

When a docker test starts the full CockroachDB source directory is "mounted" inside the docker image in location /go/src/github.com/cockroachdb/cockroach. If you place the .java files somewhere in acceptance/ (e.g. acceptance/java/) you can then access them inside the docker image from /go/src/github.com/cockroachdb/cockroach/acceptance/java. Let me know if you have more questions.

@rohangulati rohangulati mentioned this issue Nov 12, 2016
Closed
rohangulati added a commit to rohangulati/cockroach that referenced this issue Nov 13, 2016
@rohangulati
Refactors java acceptance test
0a27601 
Fixes cockroachdb#4591

This moves the java acceptance test to separate java files inside `acceptance/java` package to ease editing. Positive java test cases live under `acceptance/java/success` and negative test cases under `acceptance/java/fail`. The go test code invokes a bash scripts 'run_java_test.sh' which compiles and runs 'TestRunner.java` inside the test docker container. 'TestRunner.java` is responsible for running all java test under 'acceptance/java/success` and 'acceptance/java/fail'. It does so by accepting 2 command line arguments [expectedOutcome] [test_dir]. 'TestRunner' also makes sure that all generated files (.class and .pk8) files are cleaned up.
@spencerkimball spencerkimball removed this from the 1.0 milestone Mar 29, 2017
@dianasaur323 dianasaur323 added this to the Later milestone Apr 20, 2017
@tamird tamird mentioned this issue Oct 17, 2017
Merged
justinj pushed a commit to justinj/cockroach that referenced this issue Oct 18, 2017
build: allow directories for acceptance tests
7703963 
Closes cockroachdb#4591.

🔬🐶, this commit lets acceptance tests specify a directory which should
be mounted.

Included is a test of jdbc which translates the old string literal test
into one which has a directory structure and can be opened in IntelliJ.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-cleanup Tech debt, refactors, loose ends, etc. Solution not expected to significantly change behavior. E-easy Easy issue to tackle, requires little or no CockroachDB experience
Projects
None yet
Milestone
Later
Development

No branches or pull requests
5 participants
@knz @a6802739 @rohangulati @dianasaur323 @spencerkimball