search

flightstats / hub

fault tolerant, highly available service for data storage and distribution
http://www.flightstats.com
MIT License
103 stars 35 forks source link

Start helmifying system tests #1203

Closed lkemmerer closed 5 years ago

lkemmerer commented 5 years ago

This is not ready to merge. I need to make a few tweaks, but having folks take a look at it now should help streamline getting all the system test stuff in. It'll let me incorporate feedback while I continue working and get y'all familiar with the code so that Paul can PR hist tests against this branch.

lkemmerer commented 5 years ago

K, AFAIK, this is ready to go. Let me know if there's anything that hasn't been addressed to your satisfaction, @manjula-chikkanagappa and @Paul-Hess.

lkemmerer commented 5 years ago

I use gradle properties to pass in the hub url for running system tests on Jenkins against whichever cluster is passed in as a parameter. This seemed like a more straightforward method than recreating/modifying the properties file during a test run. The way I implemented it will allow Jenkins to generically override any properties rather than just the hub url, which decreases the number of places we have the property names hard coded (so just the jenkins job, the constants file, and the properties file, rather than all that AND the build.gradle file).

lkemmerer commented 5 years ago

It looks like I could use a -D (system properties) on the gradle command rather than -P (project properties), which I think should take out that block in the build file...

manjula-chikkanagappa commented 5 years ago

It looks like I could use a -D (system properties) on the gradle command rather than -P (project properties), which I think should take out that block in the build file...

that sounds so much better!

lkemmerer commented 5 years ago

I'll double check that it works, but I think it should. Not sure why I didn't do that in the first place.

lkemmerer commented 5 years ago

Hm. Switching to -D doesn't work without adding systemProperties System.properties to the (possibly with the findAll unless we want to pass all jvm props over) to the build file. According to this ancient stack overflow answer, it might be because the tests are forked into a new JVM and those properties aren't carried over: https://stackoverflow.com/questions/21406265/how-to-give-system-property-to-my-test-via-gradle-and-d @manjula-chikkanagappa If you prefer using -D and System.properties instead of project properties, I'm happy to make that change. I don't have a preference.

manjula-chikkanagappa commented 5 years ago

Hm. Switching to -D doesn't work without adding systemProperties System.properties to the (possibly with the findAll unless we want to pass all jvm props over) to the build file. According to this ancient stack overflow answer, it might be because the tests are forked into a new JVM and those properties aren't carried over: https://stackoverflow.com/questions/21406265/how-to-give-system-property-to-my-test-via-gradle-and-d @manjula-chikkanagappa If you prefer using -D and System.properties instead of project properties, I'm happy to make that change. I don't have a preference.

That would be nice Laurie. These are JVM properties and not gradle properties as pointed out in the stackoverflow link.

lkemmerer commented 5 years ago

got a verbal/slack 👍 from Paul, too.