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.

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

Make generated code pretty to read (and write) #280

Merged
merged 7 commits into from
Oct 18, 2024

Conversation

atifaziz
Copy link
Contributor

@atifaziz atifaziz commented Oct 16, 2024

This PR adds snapshot-based testing of the generated code and then goes to make the generated code (hopefully) a joy to read and write by formatting with conventional spacing and indentation. This helps with readability for users who may want to step into the code in the debugger and also allows changes to the generate code to be Git-tracked, fully tested and reviewable as part of future PRs. While the integration tests covered the runtime behaviour, this helps to ensure that accidental source changes don't slip by without notice.

The new tests exercise the generated code for all the Python files from the integration tests:

  • CSnakes.Tests
    • PythonStaticGeneratorTests
      • FormatClassFromMethods
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_args.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_basic.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_buffer.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_defaults.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_dependency.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_dicts.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_exceptions.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_false_returns.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_generators.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_keywords.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_none.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_reload.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_reserved.py")
        • FormatClassFromMethods(resourceName: "CSnakes.Tests.python.test_tuples.py")

The snapshot testing is leveraged from Shouldly using its ShouldMatchApproved.

This will also eventually help with generating prettier code via a CLI (#192).

Comment on lines +50 to +63
catch (FileNotFoundException ex) when (ex.FileName is { } fn
&& fn.Contains(".received.", StringComparison.OrdinalIgnoreCase))
{
// `ShouldMatchApproved` deletes the received file when the condition is met:
// https://github.com/shouldly/shouldly/blob/4.2.1/src/Shouldly/ShouldlyExtensionMethods/ShouldMatchApprovedTestExtensions.cs#L70
//
// `File.Delete` is documented to never throw an exception if the file doesn't exist:
//
// > If the file to be deleted does not exist, no exception is thrown. Source:
// > https://learn.microsoft.com/en-us/dotnet/api/system.io.file.delete?view=net-8.0#remarks
//
// However, `FileNotFoundException` has been observed on some platforms during CI runs
// so we catch it and should be harmless to ignore.
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers: See failing run #31639380191 for why this ugly hack was needed; baffles the mind, frankly.

@tonybaloney tonybaloney self-requested a review October 17, 2024 09:23
@tonybaloney tonybaloney merged commit 93a5434 into tonybaloney:main Oct 18, 2024
37 checks passed
@atifaziz atifaziz deleted the pretty-code-gen-snapshots branch October 18, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants