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

improve ability of developers to run integration tests #4369

Closed
pameyer opened this issue Dec 8, 2017 · 15 comments
Closed

improve ability of developers to run integration tests #4369

pameyer opened this issue Dec 8, 2017 · 15 comments
Assignees

Comments

@pameyer
Copy link
Contributor

pameyer commented Dec 8, 2017

Integration tests at https://build.hmdc.harvard.edu:8443/job/phoenix.dataverse.org-apitest-develop/ are useful; documentation of how to run these tests on a local system can be confusing for a new developer.

possibly related: #3938, #1936, #3959 , #2443

@pdurbin
Copy link
Member

pdurbin commented Dec 12, 2017

@pameyer thanks for pull request #4370 ! I'm copying and pasting a comment I just left there because we tend to favor discussion in issues rather than pull requests:

@pameyer overall, this is a very interesting contribution. I was going to try to execute your code but I'm blocked on not knowing what "surgery" steps are necessary and am much more interested in a fully automated solution than one with manual steps. Both Vagrant and our existing Docker files set up Glassfish and Solr in an automated way. Can we do the same in this "all in one" solution? Also, there seems to be some duplication of files, such as the Solr schema.xml and the post script I use to deploy to the phoenix server. Let's talk some more about definition of done for this issue. Perhaps all we need is more documentation about the surgery? Thanks!

@pameyer
Copy link
Contributor Author

pameyer commented Dec 12, 2017

  • the "surgery" steps are the ones from the guides (and Vagrant / existing Docker files) for glassfish ans solr. My thinking for not doing those here is that they 1) introduce additional points of failure into the process 2) send un-necessary and easily-avoidable network traffic on every build. This might be something that could be moved into the setup script, but I was shooting for an incremental improvement.
  • duplication of the solr schema.xml is an oversight (hangover from that being something frequently customized); probably reasonable to fix.
  • post script needed to be modified, because I was reverse engineering "deploy to phoenix" to get to "do stuff necessary to setup data for integration tests"; any of the phoenix scripts that weren't modified should be used in their original forms/locations.

@pdurbin
Copy link
Member

pdurbin commented Dec 12, 2017

@pameyer talked this out a bit at http://irclog.iq.harvard.edu/dataverse/2017-12-12 and plan to meet up tomorrow if time allows. It would do me good to see a demo.

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

I got a great brain dump from @pameyer as well as some "deps" (dependencies, two tarballs) I need to try to execute the code in his pull request. The Day of Meetings prevented us from talking more about running tests on developers' laptops vs. Jenkins but I have a much better understanding of where @pameyer is coming from and I like the way he thinks!

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

@pameyer thanks for providing me with glassfish4dv.tgz and solr-4.6.0dv.tgz. I've been leaving comments in your pull request (#4370). While I don't think there's any harm in merging it, I'm somewhat troubled that running the REST Assured tests is hanging here for me:

------htmlStr4: com.jayway.restassured.internal.RestAssuredResponseImpl@70d577e9
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 3.004 sec
Running edu.harvard.iq.dataverse.api.FileMetadataIT
Dec 14, 2017 9:56:11 AM edu.harvard.iq.dataverse.api.UtilIT getRestAssuredBaseUri
INFO: Base URL for tests: http://localhost:8083
TOKEN: c24070cf-9f97-4b42-955c-801a74dde388
DATAVERSE: http://localhost:8083/dataverse/271f6688
{
    "status": "ERROR",
    "message": "Validation failed: Directory Name cannot contain leading or trailing file separators. Invalid value: 'data/subdir1/'."
}

Oh, also, your pull request includes a pg_hba.conf for postgres but we already have it at https://github.com/IQSS/dataverse/blob/v4.8.4/conf/vagrant/var/lib/pgsql/data/pg_hba.conf so it might be nice to re-use it if we can.

@donsizemore since the pull request uses CentOS 7 rather than CentOS 6, you might be interesting in taking a look as you poke at #4384.

The last thing I'll say for now is that I still want to talk to @pameyer about how we can execute all this great code automatically from Jenkins or Travis or some other continuous integration service. This pull request does definitely add value in the sense that it forces the user to run the installer. This differs from the phoenix setup where I ran a shell script that was somewhat equivalent to the installer a couple years ago. Right now we already say "To run more than one test at a time, separate by commas" over at http://guides.dataverse.org/en/4.8.4/developers/testing.html#integration-tests but developers are not in the habit of running all the tests on their laptops. I think it's too much to ask. These tests should be run automatically on a server. I think this pull request is incremental progress toward that goal, something @bjonnh and I have talked about.

Sorry so rambly.

@pameyer
Copy link
Contributor Author

pameyer commented Dec 14, 2017

@pdurbin Longer term, I'd been thinking it would make sense to have a Jenkins declarative pipeline that would build everything (war, installer zip, docker images), run the unit and integration tests (and browser-based tests if/when they become available), and generate coverage reports (I'm pretty sure coveralls is missing integration test coverage).

I'd missed that other copy of pg_hba.conf.

It may be possible to extend Travis to build images / run integration tests, but it's an approach I'm less familiar with.

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

I would be nice to know why conf/docker-aio/dv/postgresql.conf is present.

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

Note to self: to get a shell in the Docker container: docker exec -it adb505218f64 bash

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

server.log is showing this which I believe is normal because it's what we're testing above:

[2017-12-14T14:56:12.383+0000] [glassfish 4.1] [WARNING] [] [edu.harvard.iq.dvn.core.index.DOIEZIdServiceBean] [tid: _ThreadID=29 _ThreadName=http-listener-1(2)] [timeMillis: 1513263372383] [levelValue: 900] [[
  message bad request - unrecognized identifier scheme]]

[2017-12-14T14:56:12.414+0000] [glassfish 4.1] [INFO] [] [edu.harvard.iq.dataverse.DatasetVersion] [tid: _ThreadID=29 _ThreadName=http-listener-1(2)] [timeMillis: 1513263372414] [levelValue: 800] [[
  Constraint violation found in FileMetadata. Directory Name cannot contain leading or trailing file separators. The invalid value is "data/subdir1/".]]

[2017-12-14T14:56:12.476+0000] [glassfish 4.1] [INFO] [] [] [tid: _ThreadID=30 _ThreadName=Thread-8] [timeMillis: 1513263372476] [levelValue: 800] [[
  vals = [edu.harvard.iq.dataverse.ControlledVocabularyValue[ id=12 ]], contains: false]]

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

I just ran the test suite for the second time and this time it worked (I'm not sure why):

Results :

Tests run: 45, Failures: 0, Errors: 0, Skipped: 0

[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 01:30 min
[INFO] Finished at: 2017-12-14T11:43:38-05:00
[INFO] Final Memory: 55M/705M
[INFO] ------------------------------------------------------------------------

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

@pameyer just had a very productive conversation lasting over an hour. I'm too lazy to type up a todo list but my chicken scratch will remind me:

img_20171214_124527

@pdurbin
Copy link
Member

pdurbin commented Dec 14, 2017

@pameyer it occurs to me that perhaps the subject of issue should include QA. Right now it sounds quite oriented toward developers.

@pdurbin
Copy link
Member

pdurbin commented Dec 15, 2017

In bc7aa68 I explained in the Dev Guide that there are now two different uses of Docker once pull request #4370 is merged:

  • Docker images for Minishift/Openshift (merged in pull request 4040 docker openshift #4168)
  • An "all in one" Docker image that can be used to run integration tests against

@pameyer hopefully it's clear enough from that commit where you can add additional documentation if you are so inclined.

Here's the todo list @pameyer and I came up with yesterday:

  • Write script to create glassfish4dv.tgz.
  • Write script to create solr-4.6.0dv.tgz.
  • Remove conf/docker-aio/dv/postgresql.conf or document why it's needed (nice to have, not strictly needed to pass code review).

The main goal of this current effort is to make it easier to run the full REST Assured API Test suite before merging a pull request. Obviously, not every pull request needs to have the full test suite run against it. Sometimes we only make minor documentation changes. The target environment for running the test suite is a Mac but in the future we can imagine further automation where Jenkins kicks off testing on a Linux box, probably in the cloud.

We also talked about the value add that this effort represents. Here's my take on the value add, most value first:

  • Relatively easy way to run full API suite before merging (main goal).
  • Faster way to test the installer script than in Vagrant. I haven't benchmarked this but I trust Pete on this.
  • Easier for Dataverse developers to get familiar with Docker without needing to also run orchestration software such as Minishift/OpenShift/Kubernetes.
  • A way to enumerate all the *IT.java tests we run on the phoenix server (more get added from time to time)
  • Developers interested in running the full integration suite may find this easier than doing all the setup described in the dev guide.
  • Perhaps Windows dev will like it? (There is not much interested in Docker in How to get Dataverse running on a Windows computer for development #3927 so far.)
  • As of this writing and for the last six months of so, I can't run the full test suite on my installation of Glassfish on my Mac without Glassfish restarting on me. I'm not sure why. They all work in the Docker "all in one" image with no restart. I'm not sure if anyone else is affected by this.

Finally, in a way, this issue is the real #3938 in my mind. That is to say, it's focused much more on Docker alone without adding Minishift/OpenShift into the mix.

@pameyer I'm taking myself off this issue since I made the tweaks to the documentation I wanted to. I'm happy to pitch in if you need any help with the todo list above. I also put a but in @bjonnh 's ear over at http://irclog.iq.harvard.edu/dataverse/2017-12-15#i_61497 . Good work. I like the direction this effort is taking.

@pdurbin
Copy link
Member

pdurbin commented Dec 19, 2017

@pameyer did all the stuff we talked about (thanks!) so I moved this issue and pull request #4370 to QA. The starting point for testing is a line in the "Development Environment" section of the Dev Guide that says this:

The “all in one” Docker files are in conf/docker-aio and you should follow the readme in that directory for more information on how to use them.

That readme can be found here: https://github.com/IQSS/dataverse/tree/d0f4c68d2ad26928fd185018d91ecdeb70c91e9e/conf/docker-aio

I just ran through all the step as of the commit above and got BUILD SUCCESS with a "total time" of just over one and half minutes for all the API tests mentioned in the readme.

@kcondon kcondon self-assigned this Dec 20, 2017
@kcondon kcondon closed this as completed Jan 8, 2018
@kcondon kcondon removed the Status: QA label Jan 8, 2018
@pdurbin
Copy link
Member

pdurbin commented Mar 2, 2018

Just a heads up that in e796935 I'm fixing up the readme for the "all in one" Docker environment and explaining how it fits into our testing strategy.

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

No branches or pull requests

4 participants