Adds integration tests for code generator #95

desaikd · 2024-04-08T17:17:27Z

Issue #88:

Description of changes:

This PR works on adding pre-defined code gen test projects and roundtrip tests for the generated code.

List of changes:

Renames beta-subcommand feature to experimental-code-gen (This makes it consistent with ion-rs on naming experimental feature. Also, since this feature is only used for code gen, using beta wouldn't be specific)
Changes in struct and class template to sort field based on field name to generate consistent constructor signature for generated code.
Adds code-gen-projects that has some pre-defined code generation projects for both Java(gradle project code-gen-demo) and Rust(cargo crate code-gene-demo).
- Adds input Ion files for testing (/code-gen-proejcts/input)
- Adds schema files for testing (/code-gen-proejcts/schema)
- Adds cargo crate in rust/code-gen-demo for testing in Rust
- Adds gradle project in java/code-gen-demo for testing in Java
- Adds build process in both Rust and java projects, to generate code
  while building and use it for tetsing
- Adds tests in both Rust and Java projects
Adds code-gen-tests in ion-cli to invoke both Rust and Java
projects for testing
-Modifies CI workflow
- Workflow modified to run code gen tests separately
- Currently code gen tests only runs for ubuntu (Since code generation tests takes longer to clean previously generated code, generate code and then test for both Java and Rust, changed it to be just ubuntu. Also, seeing error in running gradle project in GitHub Actions for windows platform returns error which needs to be investigated: https://github.com/desaikd/ion-cli/actions/runs/8574599832/job/23501774774)
Adds changes to sort schema file paths to preserve anonymous type names

Sample of running tests on `ion-cli` for code generator:

   Running tests/code-gen-tests.rs 

running 2 tests
     Removed X files, YMiB total
Starting a Gradle Daemon (subsequent builds will be faster)
> Task :clean

> Task :ionCodegen
Started generating code...
Code generation complete successfully!
Path to generated code: ion-cli/code-gen-projects/java/code-gen-demo/build/generated/java

> Task :compileJava
> Task :processResources NO-SOURCE
> Task :classes
> Task :compileTestJava
> Task :processTestResources NO-SOURCE
> Task :testClasses
> Task :test

BUILD SUCCESSFUL in Zs
5 actionable tasks: 5 executed
test roundtrip_tests_for_generated_code_gradle ... ok

running 3 tests
test tests::it_works ... ok
test tests::test_roundtrip_generated_code_structs_with_fields ... ok
test tests::test_roundtrip_generated_code_nested_structs ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 
....
test roundtrip_tests_for_generated_code_cargo ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0

Tests

Adds code-gen-tests in ion-cli to invoke both Rust and Java
Adds tests in code-gen-projects for both Rust and Java projects

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

* Adds input Ion files for testing (/code-gen-proejcts/input) * Adds schema files for testing (/code-gen-proejcts/schema) * Adds cargo crate in rust/code-gen-demo for testing in Rust * Adds gradle project in java/code-gen-demo for testing in Java * Adds build process in both Rust and java projects, to generate code while building and use it for tetsing * Adds tests in both Rust and Java projects

* Adds `code-gen-tests` in `ion-cli` to invoke both Rust and Java projects for testing

* Workflow modfiied to run code gene tests separately * Currently code gen tests only runs for ubuntu

desaikd · 2024-04-08T17:20:21Z

.github/workflows/build.yml

@@ -33,7 +33,13 @@ jobs:
        uses: actions-rs/cargo@v1
        with:
          command: test
-          args: --verbose --workspace --all-features
+          args: --verbose --workspace
+      - name: Cargo Test - Code generation tests


(🗺️ PR tour) Adds GitHub Action for code generation tests. This currently only runs on ubuntu to reduce time for running tests on code generation. code generation tests involve 3 steps:

Cleans the predefined Java and Rust project targets.

Generates code to these predefined projects using ion-cli subcommand generate

Runs tests defined on these Java and Rust projects.

desaikd · 2024-04-08T17:21:20Z

Cargo.toml

@@ -40,7 +40,7 @@ tempfile = "~3.5.0"

 [features]
 default = []
-beta-subcommands = ["dep:tera", "dep:convert_case"]
+experimental-code-gen = ["dep:tera", "dep:convert_case"]


(🗺️ PR tour) Renamed beta-subcommand feature to experimental-code-gen as its only used for code generation code and uses experimental-XYZ signature similar to ion-rs.

desaikd · 2024-04-08T17:22:51Z

code-gen-projects/java/code-gen-demo/src/test/java/org/example/CodeGenTest.java

+import static org.junit.jupiter.api.Assertions.*;
+import java.util.ArrayList;
+
+class CodeGenTest {


(🗺️ Pr tour) This class has tests for the generated code based on schema files in /code-gen-projects/schema/*.

Currently Java tests have manual tests for getter and setters but this can be changed to use roundtrip tests once read-write API generation is supported in Java.

desaikd · 2024-04-08T17:24:03Z

code-gen-projects/rust/code-gen-demo/src/lib.rs

+    }
+
+    #[test]
+    fn test_roundtrip_generated_code_structs_with_fields() -> IonResult<()> {


(🗺️ PR tour) Following are tests for the generated code in Rust based on /code-gen-projects/schema/*(schema files) and code-gen-projects/input/*(input files).

desaikd · 2024-04-08T17:26:27Z

code-gen-projects/rust/code-gen-demo/src/lib.rs

+        structs_with_fields.write_to(&mut text_writer)?;
+        text_writer.flush()?;
+        // compare given Ion value with round tripped Ion value written using abstract data type's `write_to` API
+        assert_eq!(


(🗺️ PR tour) Roundtrip test has following steps:

Read Ion data into data model(e.g. StructWithFields) using read_from API.

Write data model as Ion using write_to API.

Assert both expected Ion data and Ion data written by data model creates same Element.

desaikd · 2024-04-08T17:28:15Z

src/bin/ion/commands/beta/generate/templates/java/class.templ

@@ -6,7 +6,7 @@ public final class {{ target_kind_name }} {
       private final {{ field.value }} {{ field.name | camel }};
 {% endfor %}

-    public {{ target_kind_name }}({% for field in fields -%}{{ field.value }} {{ field.name | camel }}{% if not loop.last %},{% endif %}{% endfor %}) {
+    public {{ target_kind_name }}({% for field in fields | sort(attribute="name") -%}{{ field.value }} {{ field.name | camel }}{% if not loop.last %},{% endif %}{% endfor %}) {


(🗺️ PR tour) Sorting fields by field name ensures consistent constructor API generation for data model.

We should not expose the constructor, if we can help it. Unless we generate overloaded constructors, using a constructor means that adding a new optional field is backwards incompatible. (And if you have two optional fields with the same type, then you've got a pretty good chance of having a method signature conflict.) On the other hand, if we use builders, then the generated order doesn't matter for us or for consumers of the generated code.

I don't think it needs to be solved now, but it does need to be called out as a current limitation, and it needs to be fixed before we can say that it is ready for customers to use.

Right. Since currently the Java code generation doesn't support read-write API generation, I had to use the constructors for adding manual tests. I will create an issue for the work on builder generation support.
Reference issue: #97

desaikd · 2024-04-08T17:30:09Z

tests/code-gen-tests.rs

+
+#[cfg(feature = "experimental-code-gen")]
+#[test]
+fn roundtrip_tests_for_generated_code_gradle() -> Result<()> {


(🗺️ PR tour) These tests will invoke the predefined projects in code-gen-projects, generates code based on their build process(build process invokes ion-cli generate subcommand) and then runs tests for these predfined projects.

nirosys

Question about the included Foo example, but otherwise lgtm.

code-gen-projects/java/code-gen-demo/src/main/java/org/example/Foo.java

popematt · 2024-04-09T22:42:49Z

code-gen-projects/java/code-gen-demo/build.gradle.kts

+        inputs.files(ionSchemaSourceCodeDir)
+        outputs.file(generatedIonSchemaModelDir)
+
+        val ionCli = "../../../target/debug/ion"


You can pass the executable path using an environment variable. E.g.:

Set the path as an environment variable in the Rust integration test: https://github.com/popematt/ion-cli/blob/a24ecd23d659f6c0142b20bbc8a4771ac6a01c80/tests/codegen-tests.rs#L59-L65

Access the path from the gradle build script: https://github.com/popematt/ion-cli/blob/a24ecd23d659f6c0142b20bbc8a4771ac6a01c80/codegen-test-projects/java/build.gradle.kts#L33

Follow-up PR: #98

popematt · 2024-04-09T22:47:43Z

code-gen-projects/rust/code-gen-demo/build.rs

+    let out_dir = env::var("OUT_DIR").unwrap();
+
+    // Invoke cargo CLI
+    let ion_cli = "../../../target/debug/ion";


You should pass in the path from the integration tests.

This is not something that customers can copy. Getting it from the environment variable or the user path is something that customers can copy.

Hard coded paths like this are brittle.

This test project should not have to know anything about the outer project that contains it.

Thanks for the explanation, I have modified according to your suggestions in a Follow-up PR: #98

desaikd added 7 commits April 5, 2024 13:28

Renames feature beta-subcommand to experimental-code-gen

bb9c488

Changes to preserve signature of constructor definition by field order

f8a68c4

Adds code-gen-tests in ion-cli

909b0c5

* Adds `code-gen-tests` in `ion-cli` to invoke both Rust and Java projects for testing

Modifies CI workflow

baf581c

* Workflow modfiied to run code gene tests separately * Currently code gen tests only runs for ubuntu

Adds clippy suggestion

282330b

Adds changes to sort schema file paths to preserve anonymous type names

8913607

desaikd requested a review from nirosys April 8, 2024 17:32

desaikd commented Apr 8, 2024

View reviewed changes

nirosys approved these changes Apr 9, 2024

View reviewed changes

code-gen-projects/java/code-gen-demo/src/main/java/org/example/Foo.java Outdated Show resolved Hide resolved

adds changes to remove Foo example in gradle proejct

2184ed9

desaikd mentioned this pull request Apr 9, 2024

Add support to generate builders in code generation #97

Open

2 tasks

desaikd merged commit fe2a279 into amazon-ion:master Apr 9, 2024
4 checks passed

desaikd mentioned this pull request Apr 9, 2024

Define a test runner for roundtrip generated code tests #88

Closed

popematt requested changes Apr 9, 2024

View reviewed changes

desaikd mentioned this pull request Apr 9, 2024

Adds changes for code generation tests configuration #98

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds integration tests for code generator #95

Adds integration tests for code generator #95

desaikd commented Apr 8, 2024 •

edited

Loading

desaikd Apr 8, 2024

desaikd Apr 8, 2024

desaikd Apr 8, 2024

desaikd Apr 8, 2024

desaikd Apr 8, 2024

desaikd Apr 8, 2024

desaikd Apr 8, 2024

popematt Apr 9, 2024

desaikd Apr 9, 2024

desaikd Apr 8, 2024

nirosys left a comment

popematt Apr 9, 2024 •

edited

Loading

desaikd Apr 9, 2024 •

edited

Loading

popematt Apr 9, 2024

desaikd Apr 9, 2024

Adds integration tests for code generator #95

Adds integration tests for code generator #95

Conversation

desaikd commented Apr 8, 2024 • edited Loading

Issue #88:

Description of changes:

List of changes:

Sample of running tests on ion-cli for code generator:

Tests

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nirosys left a comment

Choose a reason for hiding this comment

popematt Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

desaikd Apr 9, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

desaikd commented Apr 8, 2024 •

edited

Loading

Sample of running tests on `ion-cli` for code generator:

popematt Apr 9, 2024 •

edited

Loading

desaikd Apr 9, 2024 •

edited

Loading