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
base: master
Are you sure you want to change the base?
Changes from all commits
c5067e6
440a0b0
9d038c9
e988b51
5118d06
3ccb3e8
31d8d48
41c01ba
dd36f31
c03e53e
5377d9a
0d3a6bc
ead428c
a207daf
a83ee9a
d312555
ff99679
a58e36c
8f6b74f
92331c8
3f4168e
04ee2f7
cc6514d
b19227e
64f6516
4ff1e11
b8b3e01
bc3e523
fd06b2d
4cc68f1
57fcebd
f05f63a
79b4c61
3d7a381
d6ca75d
1a7a9dc
bdfbc8f
a8fa1a4
3e95123
e4479b2
eacdb50
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.1' | ||
def agronaVersionRange = '[1.21.1,2.0[' // allow any release >= 1.21.1 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