-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fortran serialization codegen tests #198
Merged
samkellerhals
merged 20 commits into
fortran-serialisation
from
fortran-serialization-codegen-tests
May 9, 2023
Merged
Changes from 19 commits
Commits
Show all changes
20 commits
Select commit
Hold shift + click to select a range
511ae43
No dependency test created
ChristopherBignamini 6d5034e
Missing dependency test added
ChristopherBignamini 853c7bc
Added test for non existing granule input file
ChristopherBignamini d480745
Fix for source with no external dependencies
ChristopherBignamini 173f2de
Serialization directives generation test created
ChristopherBignamini 55d6591
Add small fixes
samkellerhals 48ee72b
Serialization directives generation test created for diffusion granule
ChristopherBignamini 43d4ce2
Useless import removed
ChristopherBignamini 7e87416
Useless import removed
ChristopherBignamini 9e60086
Merge branch 'fortran-serialisation' into fortran-serialization-codeg…
ChristopherBignamini 6e9f16c
New interface adopted
ChristopherBignamini f3596a9
Codegen fixtures created
ChristopherBignamini 6f106b5
Serialization directives reference file created
ChristopherBignamini 1e71448
Fix tests
samkellerhals ba12eb5
Merge branch 'fortran-serialisation' into fortran-serialisation-cli-t…
ChristopherBignamini 34fc8dc
Merge branch 'fortran-serialisation-cli-tests' of https://github.com/…
ChristopherBignamini adb2d4b
Merge branch 'fortran-serialisation-cli-tests' into fortran-serializa…
ChristopherBignamini 645f312
Merge branch 'fortran-serialisation' into fortran-serialization-codeg…
ChristopherBignamini 66952a0
Fix tests
samkellerhals e1859cb
Remove print statement
samkellerhals File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Could you also add one test case for a fortran file which does have dependencies and where the dependencies include derived types so we can test they are being generated correctly?
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.
I added a test_deserialiser_directives_diffusion_codegen test where I check if the serialization directives are correctly generated for the diffusion case, so the derived types serialization are included. I'm not sure this is what you expected, maybe I can create a smaller more specific test...
I have a question concerning the reference code fixtures: in the shorter case of the test_deserialiser_directives_no_deps_codegen test, where I check the generation of the serialization directives for a no-deps small source code, I create the GeneratedCode items and save them in a list, exactly as we do in the serialization code. In the diffusion case, instead, I saved the reference code in a file and then compare the file content to the generated code, after a conversion into a string of the GeneratedCode list (because creating the GeneratedCode instances is a bit complex giving the source code size). Should I unify the two approaches?
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.
For the small reference case it makes sense to define the generated code fixture like you did it. I agree that for more complex cases such as when defining the expected code for the diffusion granule, a better approach would be to define the expected code in an external file. Here a compromise would be to for example only check the first savepoint since otherwise the file would be quite large. I've modified the test to check for only the first savepoint, since anyhow that includes all the parts we want to test.