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

Gradle pt 1: tests + coverage #722

Merged
merged 4 commits into from
Mar 28, 2024
Merged

Gradle pt 1: tests + coverage #722

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c4bf84b
Gradle pt 1: tests + coverage
MrCreosote Mar 7, 2024
b871872
Remove test.yml cruft
MrCreosote Mar 26, 2024
c5f3d89
Fix quick test coverage
MrCreosote Mar 26, 2024
6fdb23b
Alphabetize deps & update syslog4j comment
MrCreosote Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .classpath
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="server_scripts"/>
<classpathentry excluding=".gitignore" kind="src" path="src"/>
<classpathentry kind="src" path="performanceanddevelopment"/>
Xiangs18 marked this conversation as resolved.
Show resolved Hide resolved
<classpathentry kind="con" path="org.eclipse.jdt.launching.JRE_CONTAINER">
<attributes>
<attribute name="module" value="true"/>
Expand Down Expand Up @@ -52,7 +50,6 @@
<classpathentry kind="lib" path="/jars/lib/jars/kbase/auth/kbase-auth-0.4.4.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/junit/junit-4.12.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/hamcrest/hamcrest-core-1.3.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/kbase/auth2/kbase-auth2-test-fat-0.6.1.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/kafka/kafka-clients-2.1.0.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/kbase/shock/shock-client-0.1.0.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/amazon/V2/eventstream-1.0.1.jar"/>
Expand Down Expand Up @@ -109,5 +106,6 @@
<classpathentry kind="lib" path="/jars/lib/jars/amazon/V2/url-connection-client-2.17.214.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/amazon/V2/utils-2.17.214.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/kbase/common/kbase-common-0.2.0.jar"/>
<classpathentry kind="lib" path="/jars/lib/jars/kbase/auth2/kbase-auth2-test-fat-0.6.1.jar"/>
<classpathentry kind="output" path="bin"/>
</classpath>
1 change: 0 additions & 1 deletion .codecov.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,4 @@ comment:
require_changes: no

ignore:
- "build"
Xiangs18 marked this conversation as resolved.
Show resolved Hide resolved
- "deployment"
6 changes: 6 additions & 0 deletions .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#
# https://help.github.com/articles/dealing-with-line-endings/
#
# These are explicitly windows files and should use crlf
*.bat text eol=crlf

12 changes: 3 additions & 9 deletions .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@ jobs:
mongo: 'mongodb-linux-x86_64-ubuntu2204-7.0.4'
minio: '2019-05-23T00-29-34Z'
wired_tiger: 'true'
ant_test: 'test_quick_coverage'
gradle_test: 'testQuick'
# the current production setup
- java: '11'
mongo: 'mongodb-linux-x86_64-3.6.13'
minio: '2019-05-23T00-29-34Z'
wired_tiger: 'false'
ant_test: 'test_quick_coverage'
gradle_test: 'testQuick'
steps:
- uses: actions/checkout@v3

Expand Down Expand Up @@ -84,10 +84,6 @@ jobs:
# move to parent dir of homedir to install binaries etc
cd ..

# set up jars
git clone https://github.com/kbase/jars
export JARSDIR=$(pwd)/jars/lib/jars/

# set up arango
export ARANGODB_VER=3.9.1
export ARANGODB_V=39
Expand Down Expand Up @@ -142,7 +138,6 @@ jobs:
sed -i "s#^test.mongo.exe.*#test.mongo.exe=$MONGOD#" test.cfg
sed -i "s#^test.minio.exe.*#test.minio.exe=$MINIO#" test.cfg
sed -i "s#^test.mongo.useWiredTiger.*#test.mongo.useWiredTiger=${{matrix.wired_tiger}}#" test.cfg
sed -i "s#^test.jars.dir.*#test.jars.dir=$JARSDIR#" test.cfg
sed -i "s#^test.blobstore.exe.*#test.blobstore.exe=$BLOBEXE#" test.cfg
sed -i "s#^test.handleservice.dir.*#test.handleservice.dir=$HSDIR#" test.cfg
sed -i "s#^test.sampleservice.dir.*#test.sampleservice.dir=$SAMPLE_DIR#" test.cfg
Expand All @@ -153,8 +148,7 @@ jobs:
- name: Run tests
shell: bash
run: |
ant javadoc
ant ${{matrix.ant_test}}
./gradlew ${{matrix.gradle_test}}

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v3
Expand Down
245 changes: 245 additions & 0 deletions build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,245 @@
/*
* This file was generated by the Gradle 'init' task.
*/

// TODO TEST switch to Kotlin DSL which is now the default and apparently better
// TODO TEST avoid early configuration, see
// https://docs.gradle.org/current/userguide/task_configuration_avoidance.html
// need to avoid withType as well apparently?

plugins {
id 'java'
id 'war'
id 'jacoco'
id 'org.ajoberstar.grgit' version '4.1.1'
id 'com.github.johnrengelman.shadow' version '8.1.1'
Copy link
Collaborator

@ialarmedalien ialarmedalien Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

what are these two for?

org.ajoberstar.grgit - some git jiggery-pokery
com.github.johnrengelman.shadow - in place of using gradle to build the jar

is that about right?

Copy link
Member Author

Choose a reason for hiding this comment

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

First one will be for pulling the git commit for building into the server, so we don't have to rely on kb-sdk compile. Second one will be for building a shadow jar, which Gradle can't do out of the box

Copy link
Member Author

Choose a reason for hiding this comment

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

They're both used in auth2 if you want to see examples

}

// TODO NOW test shadow jar works in groups - remove JNA - commit 2
// TODO NOW get docserver working in WAR and test in docker-compose <- edit to start test server - commit 3
// TODO NOW client jar - commit 4
// TODO NOW client jar javadoc - needs java_common source - commit 4
// TODO NOW schema updater script - commit 5
// TODO NOW sdk-compile all, java, and docs - commit 6
// TODO NOW handle the git commit the same way as auth does - commit 7
// TODO NOW delete build.xml, , and Makefile - commit 8
// TODO NOW run tests from Eclipse w/o specifying classpath manually & remove sourceSets & claspath - commit 9

repositories {
mavenCentral()
}
compileJava {
options.release = 11
}

tasks.withType(Test) {
/*
* TODO TEST Figure out why tests fail without this and remove. Might have something to do
* with the stfuLoggers() call in many of the tests, might kill logging for tests that
* require it
* Although it seems to make Mongo start up correctly as well which is odd
*/
forkEvery = 1
/*
* TODO TEST split tests into mongo wrapper tests & all other tests (incl. integration).
* Set up GHA to run the non-mongo tests with a single version of mongo and run the
* mongo tests with matrixed mongo versions. Combine coverage at the end somehow
*/
systemProperty "test.cfg", "./test.cfg"
maxHeapSize = "3G"
testLogging {
exceptionFormat = 'full'
showStandardStreams = true
}
filter {
// gradle thinks that classes annotated with @RunWith are tests
excludeTestsMatching "*Tester"
}
}

tasks.withType(JacocoReport) {
reports {
xml.required = true
csv.required = true
}
}

test {
finalizedBy jacocoTestReport
}

task testQuick(type: Test) {
// for Gradle 9.0 compatibility
testClassesDirs = testing.suites.test.sources.output.classesDirs
classpath = testing.suites.test.sources.runtimeClasspath
Comment on lines +73 to +74
Copy link
Collaborator

@Xiangs18 Xiangs18 Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Are these default env vars? They are not used elsewhere in this script.

Copy link
Member Author

Choose a reason for hiding this comment

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

They're going to be required in Gradle 9 for non-default test tasks to work

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍


filter {
excludeTestsMatching "*LongTest"
}
finalizedBy "jacocoTestQuickReport" // must be a string, see TODOs at head of file
}

task jacocoTestQuickReport(type: JacocoReport, dependsOn: testQuick) {
sourceSets sourceSets.main
executionData(testQuick)
}

// Custom java project layout
sourceSets {
main {
java {
srcDirs = ["src"]
exclude '**/test/**'
}
}
test {
java {
srcDirs = ["src"]
include '**/test/**'
Copy link
Collaborator

Choose a reason for hiding this comment

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

won't this automatically be included?

Copy link
Member Author

Choose a reason for hiding this comment

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

That line is to exclude everything that's not in **/test/**. This whole block should go away later when I move the files around to match Gradle default expectations

}
resources {
srcDirs = ["src"]
include "**/*.properties"
include '**/*.spec'
include '**/*instance.*'
include '**/*.instance*'
include '**/*.html'
include '**/*.css'
include '**/*.gif'
include '**/*.js'
include '**/*.png'
include '**/*.txt'
include '**/*.weirdsuffix'
Comment on lines +102 to +112
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are these for future commits? There are not many files in the test folder

Copy link
Member Author

@MrCreosote MrCreosote Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

These are all files currently used in tests, but all the tests are under the src folder

Copy link
Collaborator

@Xiangs18 Xiangs18 Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I assume this part will be deleted once we move test folders out?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, I'll try to set things up like auth so we don't need the sourceSets section at all, but that comes last

}
}
}

javadoc {
options {
links "https://docs.oracle.com/en/java/javase/11/docs/api/"
links "https://www.javadoc.io/doc/com.fasterxml.jackson.core/jackson-core/2.9.9/"
links "https://www.javadoc.io/doc/com.fasterxml.jackson.core/jackson-databind/2.9.9/"
}
}

war {
webXml = file('war/web.xml')
}

configurations {
// can't directly access testImplementation, so extend and access
testimpl.extendsFrom testImplementation
}

def fromURL = { url, name ->
File file = new File("$buildDir/download/${name}.jar")
file.parentFile.mkdirs()
Copy link
Collaborator

@ialarmedalien ialarmedalien Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

does this check for directory existence first? You can do dir.exists()first if not.

Copy link
Member Author

@MrCreosote MrCreosote Mar 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It works regardless of whether the build directory exists or not, so presumably it's a mkdir -p situation

if (!file.exists()) {
new URL(url).withInputStream { downloadStream ->
file.withOutputStream { fileOut ->
fileOut << downloadStream
}
}
}
files(file.absolutePath)
Copy link
Collaborator

@Xiangs18 Xiangs18 Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What is this step for? L144

Copy link
Member Author

Choose a reason for hiding this comment

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

https://docs.gradle.org/current/javadoc/org/gradle/api/Project.html#files-java.lang.Object...-

I believe it's that. I'm still a bit of a Gradle beginner so my understanding here is that it's converting a java File into a type that the Gradle implementation understands

Copy link
Member Author

Choose a reason for hiding this comment

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

This whole function is something I just found and copied, to be perfectly honest. It works though

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

}

dependencies {

// ### General application dependencies ###

implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/handle/AbstractHandleClient-1.0.0.jar',
'AbstractHandleClient-1.0.0'
)
implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/auth/kbase-auth-0.4.4.jar',
'kbase-auth-0.4.4'
)
implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/kidl/kbase-kidl-parser-1409261812-7863aef.jar',
'kbase-kidl-parser-1409261812-7863aef'
)
implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/sample/SampleServiceClient-0.1.1.jar',
'SampleServiceClient-0.1.1'
)
implementation 'ch.qos.logback:logback-classic:1.1.2'
implementation 'com.google.guava:guava:14.0.1'
implementation 'com.github.ben-manes.caffeine:caffeine:2.9.3'
implementation 'commons-codec:commons-codec:1.8'
implementation 'commons-io:commons-io:2.4'
implementation 'com.fasterxml.jackson.core:jackson-annotations:2.9.9'
implementation 'com.fasterxml.jackson.core:jackson-core:2.9.9'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.9.9'
implementation 'info.picocli:picocli:4.6.1'
implementation 'org.apache.commons:commons-lang3:3.1'
implementation 'org.apache.kafka:kafka-clients:2.1.0'
implementation 'org.ini4j:ini4j:0.5.2'
implementation 'org.mongodb:bson:4.11.1'
implementation 'org.mongodb:bson-record-codec:4.11.1'
implementation 'org.mongodb:mongodb-driver-core:4.11.1'
implementation 'org.mongodb:mongodb-driver-sync:4.11.1'
implementation 'org.slf4j:slf4j-api:1.7.30'

// ### Server dependencies, specifically for java_common JsonServerServlet ###

implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/common/kbase-common-0.2.0.jar',
'kbase-common-0.2.0'
)
// Pull from jars repo vs a maven repository to avoid the JNA dependency
// Need to rework the java common logger to not use syslog4j at all since it's abandonware
// and has a ton of CVEs, even in the newer versions.
Comment on lines +192 to +193
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can get it from Clojars. Alter the repo config to the following:

repositories {
	mavenCentral()
	maven {
		name = "Clojars"
		url = "https://repo.clojars.org/"
	}
}

Copy link
Member Author

Choose a reason for hiding this comment

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

In the next commit I get rid of the JNA dependency since it prevents the server from working in a shadowjar, so I don't actually want to pull it from a repo since that'll force the JNA dependency. Plus it's no harder to pull it from jars. Plus plus we need to get rid of sylog4j anyway so a janky pulldown makes that more obvious. I'll update the comment to be more accurate along those lines

implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/syslog4j/syslog4j-0.9.46.jar',
'syslog4j-0.9.46'
)
implementation 'javax.annotation:javax.annotation-api:1.3.2'
implementation 'javax.servlet:servlet-api:2.5'
// joda-time is required for kbase-common and syslog4j
implementation 'joda-time:joda-time:2.2'
// needed for syslog4j
implementation 'net.java.dev.jna:jna:3.4.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is clearer just to have all the deps together in alphabetical order instead of like this with all these extra comments. Gradle can figure out the dependency tree, so the comments just slow down finding the relevant lines in the gradle file and add cruft.

Copy link
Member Author

Choose a reason for hiding this comment

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

The subsections and comments like this are for cases where Gradle can't figure out the dependency tree, e.g.

  • the java common section contains the dependencies for the kbase-common jar, which has no manifest
  • Same for the shock client section
  • amazon SDK doesn't automatically pull in the url client

As such I want to group these sections together to make it clear where the dependencies are coming from and why they're in the list.

// this is OOOOOOLD. But that probably means updating java_common
implementation 'org.eclipse.jetty.aggregate:jetty-all:7.0.0.v20091005'

// ### Blobstore / Shock client and dependencies ###

implementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/shock/shock-client-0.1.0.jar',
'shock-client-0.1.0'
)
implementation 'org.apache.httpcomponents:httpclient:4.5.9'
implementation 'org.apache.httpcomponents:httpmime:4.5.8'

// ### Amazon S3 ###

implementation('software.amazon.awssdk:s3:2.17.214') {
exclude module: 'apache-client'
exclude module: 'netty-nio-client'
}
implementation 'software.amazon.awssdk:url-connection-client:2.17.214'

// ### Test ###

testImplementation fromURL(
'https://github.com/kbase/jars/raw/master/lib/jars/kbase/auth2/kbase-auth2-test-shadow-all-0.7.0.jar',
'kbase-auth2-test-shadow-all-0.7.0'
)
testImplementation 'com.arangodb:arangodb-java-driver:6.7.2'
testImplementation 'com.github.zafarkhaja:java-semver:0.9.0'
testImplementation 'org.hamcrest:hamcrest-core:1.3'
testImplementation 'commons-lang:commons-lang:2.4'
testImplementation 'junit:junit:4.12'
testImplementation 'nl.jqno.equalsverifier:equalsverifier:3.1.10'
testImplementation 'org.mockito:mockito-core:3.0.0'

}

task showTestClassPath {
doLast {
configurations.testimpl.each { println it }
}
}

5 changes: 4 additions & 1 deletion docsource/releasenotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ VERSION: 0.14.3 (Released 2/22/2024)

UPDATES:

* The MongoDB clients have been updated to the most recent version and the service tested against Mongo 7.
* The MongoDB clients have been updated to the most recent version and the service tested
against Mongo 7.
* Gradle has replaced Ant as the build tool. As a consequence, all the built artifacts are now
located in the build directory, including the ``update_workspace_database_schema`` script.


VERSION: 0.14.2 (Released 11/9/2023)
Expand Down
Binary file added gradle/wrapper/gradle-wrapper.jar
View file
Open in desktop
Binary file not shown.
5 changes: 5 additions & 0 deletions gradle/wrapper/gradle-wrapper.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-8.6-bin.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
Loading
Loading