-
Notifications
You must be signed in to change notification settings - Fork 526
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
[C#,C++,Java] Generate DTOs for non-perf-sensitive usecases. #957
Changes from all commits
5389910
68af4f6
439ee4b
b94ef12
ea7e84b
526eac2
0a2a6e4
5af0274
fdf7742
852a0fe
92588c0
b205c4b
58b0e92
4f9d51b
77711dd
19f69a9
520cdb1
c1afd03
207ab1f
4201789
bc17a46
248eae9
7ee9265
1b98e1d
96b7032
029fb25
246e6f7
e33c6a8
f88d345
f965d2a
21cde2a
a0fc525
2195d6e
b9ed08b
b27bb68
1e3cf8f
328cc42
41f4b0e
75df462
60bd119
6e0234b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
name: Slow checks | ||
|
||
on: | ||
workflow_dispatch: | ||
branches: | ||
- '**' | ||
schedule: | ||
- cron: '0 12 * * *' | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.ref }} | ||
cancel-in-progress: true | ||
|
||
env: | ||
GRADLE_OPTS: '-Dorg.gradle.daemon=false -Dorg.gradle.java.installations.auto-detect=false -Dorg.gradle.warning.mode=fail' | ||
|
||
permissions: | ||
contents: read | ||
|
||
jobs: | ||
property-tests: | ||
name: Property tests | ||
runs-on: ubuntu-22.04 | ||
strategy: | ||
matrix: | ||
java: [ '21' ] | ||
dotnet: [ '8.0.x' ] | ||
env: | ||
DOTNET_SKIP_FIRST_TIME_EXPERIENCE: true | ||
DOTNET_CLI_TELEMETRY_OPTOUT: 1 | ||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v3 | ||
- name: Setup java | ||
uses: actions/setup-java@v3 | ||
with: | ||
distribution: 'zulu' | ||
java-version: ${{ matrix.java }} | ||
- name: Setup BUILD_JAVA_HOME & BUILD_JAVA_VERSION | ||
run: | | ||
java -Xinternalversion | ||
echo "BUILD_JAVA_HOME=${JAVA_HOME}" >> $GITHUB_ENV | ||
echo "BUILD_JAVA_VERSION=${{ matrix.java }}" >> $GITHUB_ENV | ||
- name: Setup java 8 to run the Gradle script | ||
uses: actions/setup-java@v3 | ||
with: | ||
distribution: 'zulu' | ||
java-version: 8 | ||
- name: Setup dotnet | ||
uses: actions/setup-dotnet@v2 | ||
with: | ||
dotnet-version: ${{ matrix.dotnet }} | ||
- name: Build .NET library | ||
run: ./csharp/build.sh | ||
- name: Run property tests | ||
run: ./gradlew propertyTest | ||
- name: Upload test results | ||
uses: actions/upload-artifact@v3 | ||
if: success() || failure() | ||
with: | ||
name: property-tests | ||
path: sbe-tool/build/reports/tests/propertyTest |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,7 @@ buildscript { | |
|
||
plugins { | ||
id 'java-library' | ||
id 'jvm-test-suite' | ||
id 'com.github.johnrengelman.shadow' version '8.1.1' apply false | ||
id 'com.github.ben-manes.versions' version '0.51.0' | ||
} | ||
|
@@ -57,6 +58,8 @@ def checkstyleVersion = '9.3' | |
def hamcrestVersion = '2.2' | ||
def mockitoVersion = '4.11.0' | ||
def junitVersion = '5.10.2' | ||
def jqwikVersion = '1.8.1' | ||
def jsonVersion = '20230618' | ||
def jmhVersion = '1.37' | ||
def agronaVersion = '1.21.2' | ||
def agronaVersionRange = '[1.21.2,2.0[' // allow any release >= 1.21.2 and < 2.0.0 | ||
|
@@ -164,6 +167,7 @@ jar.enabled = false | |
|
||
subprojects { | ||
apply plugin: 'java-library' | ||
apply plugin: 'jvm-test-suite' | ||
apply plugin: 'checkstyle' | ||
|
||
group = sbeGroup | ||
|
@@ -216,22 +220,34 @@ subprojects { | |
} | ||
} | ||
|
||
test { | ||
useJUnitPlatform() | ||
testing { | ||
suites { | ||
test { | ||
useJUnitJupiter junitVersion | ||
|
||
testLogging { | ||
for (def level : LogLevel.values()) | ||
{ | ||
def testLogging = get(level) | ||
testLogging.exceptionFormat = 'full' | ||
testLogging.events = ["FAILED", "STANDARD_OUT", "STANDARD_ERROR"] | ||
} | ||
} | ||
targets { | ||
all { | ||
testTask.configure { | ||
useJUnitPlatform() | ||
|
||
javaLauncher.set(toolchainLauncher) | ||
testLogging { | ||
for (def level : LogLevel.values()) | ||
{ | ||
def testLogging = get(level) | ||
testLogging.exceptionFormat = 'full' | ||
testLogging.events = ["FAILED", "STANDARD_OUT", "STANDARD_ERROR"] | ||
} | ||
} | ||
|
||
javaLauncher.set(toolchainLauncher) | ||
|
||
systemProperty 'sbe.enable.ir.precedence.checks', 'true' | ||
systemProperty 'sbe.enable.test.precedence.checks', 'true' | ||
systemProperty 'sbe.enable.ir.precedence.checks', 'true' | ||
systemProperty 'sbe.enable.test.precedence.checks', 'true' | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
|
@@ -248,11 +264,6 @@ project(':sbe-tool') { | |
prefer(agronaVersion) | ||
} | ||
} | ||
testImplementation files('build/classes/java/generated') | ||
testImplementation "org.hamcrest:hamcrest:${hamcrestVersion}" | ||
testImplementation "org.mockito:mockito-core:${mockitoVersion}" | ||
testImplementation "org.junit.jupiter:junit-jupiter-params:${junitVersion}" | ||
testRuntimeOnly "org.junit.jupiter:junit-jupiter-engine:${junitVersion}" | ||
} | ||
|
||
def generatedDir = 'build/generated-src' | ||
|
@@ -269,11 +280,55 @@ project(':sbe-tool') { | |
|
||
compileGeneratedJava { | ||
dependsOn 'generateTestCodecs' | ||
dependsOn 'generateTestDtos' | ||
classpath += sourceSets.main.runtimeClasspath | ||
} | ||
|
||
compileTestJava.dependsOn compileGeneratedJava | ||
|
||
testing { | ||
suites { | ||
test { | ||
dependencies { | ||
implementation files('build/classes/java/generated') | ||
implementation "org.hamcrest:hamcrest:${hamcrestVersion}" | ||
implementation "org.mockito:mockito-core:${mockitoVersion}" | ||
implementation "org.junit.jupiter:junit-jupiter-params:${junitVersion}" | ||
} | ||
} | ||
|
||
propertyTest(JvmTestSuite) { | ||
// We should be able to use _only_ the JQwik engine, but this issue is outstanding: | ||
// https://github.com/gradle/gradle/issues/21299 | ||
useJUnitJupiter junitVersion | ||
|
||
dependencies { | ||
implementation project() | ||
implementation("net.jqwik:jqwik:${jqwikVersion}") { | ||
// Exclude JUnit 5 dependencies that are already provided due to useJUnitJupiter | ||
exclude group: 'org.junit.platform', module: 'junit-platform-commons' | ||
exclude group: 'org.junit.platform', module: 'junit-platform-engine' | ||
} | ||
implementation "org.json:json:${jsonVersion}" | ||
} | ||
|
||
|
||
targets { | ||
all { | ||
testTask.configure { | ||
minHeapSize = '2g' | ||
maxHeapSize = '2g' | ||
|
||
javaLauncher.set(toolchainLauncher) | ||
|
||
systemProperty 'sbe.dll', "${rootProject.projectDir}/csharp/sbe-dll/bin/Release/netstandard2.0/SBE.dll" | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} | ||
|
||
tasks.register('generateTestCodecs', JavaExec) { | ||
dependsOn 'compileJava' | ||
mainClass.set('uk.co.real_logic.sbe.SbeTool') | ||
|
@@ -290,6 +345,21 @@ project(':sbe-tool') { | |
'src/test/resources/field-order-check-schema.xml'] | ||
} | ||
|
||
tasks.register('generateTestDtos', JavaExec) { | ||
dependsOn 'compileJava' | ||
mainClass.set('uk.co.real_logic.sbe.SbeTool') | ||
classpath = sourceSets.main.runtimeClasspath | ||
systemProperties( | ||
'sbe.output.dir': generatedDir, | ||
'sbe.target.language': 'java', | ||
'sbe.validation.stop.on.error': 'true', | ||
'sbe.validation.xsd': validationXsdPath, | ||
'sbe.generate.precedence.checks': 'true', | ||
'sbe.java.precedence.checks.property.name': 'sbe.enable.test.precedence.checks', | ||
'sbe.java.generate.dtos': 'true') | ||
args = ['src/test/resources/example-extension-schema.xml'] | ||
} | ||
|
||
jar { | ||
manifest.attributes( | ||
'Specification-Title': 'Simple Binary Encoding', | ||
|
@@ -735,7 +805,7 @@ tasks.register('generateCSharpCodecsWithXIncludes', JavaExec) { | |
'sbe-samples/src/main/resources/example-extension-schema.xml'] | ||
} | ||
|
||
tasks.register('generateCSharpCodecsTests', JavaExec) { | ||
tasks.register('generateCSharpTestCodecs', JavaExec) { | ||
mainClass.set('uk.co.real_logic.sbe.SbeTool') | ||
classpath = project(':sbe-tool').sourceSets.main.runtimeClasspath | ||
systemProperties( | ||
|
@@ -754,9 +824,21 @@ tasks.register('generateCSharpCodecsTests', JavaExec) { | |
'sbe-benchmarks/src/main/resources/fix-message-samples.xml'] | ||
} | ||
|
||
tasks.register('generateCSharpTestDtos', JavaExec) { | ||
mainClass.set('uk.co.real_logic.sbe.SbeTool') | ||
classpath = project(':sbe-tool').sourceSets.main.runtimeClasspath | ||
systemProperties( | ||
'sbe.output.dir': 'csharp/sbe-generated', | ||
'sbe.target.language': 'uk.co.real_logic.sbe.generation.csharp.CSharpDtos', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is better than the approach for multiple generators in Go in #951. For that one, we added an extra flag There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I struggled to choose between the two mechanisms: flag vs. generator. I opted for the generator, as it seemed less intrusive (and less likely to collide with changes in my other PR). However, at that time, I had not noticed the existing pattern for the Go generation. Using a flag avoids spinning up a new JVM and parsing the schema multiple times. Therefore, that might be a better pattern. We should be consistent in any case. I'm happy to use a flag instead. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency, I've added a system property, |
||
'sbe.xinclude.aware': 'true', | ||
'sbe.validation.xsd': validationXsdPath) | ||
args = ['sbe-samples/src/main/resources/example-extension-schema.xml'] | ||
} | ||
|
||
tasks.register('generateCSharpCodecs') { | ||
description = 'Generate csharp codecs' | ||
dependsOn 'generateCSharpCodecsTests', | ||
dependsOn 'generateCSharpTestCodecs', | ||
'generateCSharpTestDtos', | ||
'generateCSharpCodecsWithXIncludes' | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we're using
jvm-test-suites
to add new source sets in a Gradle 9 compatible manner.Here
implementation
meanstestImplementation
in old money.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proof