-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
[.NET] Add tests for GodotDisabledSourceGenerators
#88687
base: master
Are you sure you want to change the base?
[.NET] Add tests for GodotDisabledSourceGenerators
#88687
Conversation
f0a6656
to
ea9f8fc
Compare
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.
Thanks for adding tests for the GodotDisabledSourceGenerators
property.
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpSourceGeneratorVerifier.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/CSharpSourceGeneratorVerifier.cs
Outdated
Show resolved
Hide resolved
var verifier = new Test(); | ||
|
||
verifier.TestState.AnalyzerConfigFiles.Add(("/.globalconfig", $""" | ||
verifier.TestState.AnalyzerConfigFiles.Add((Constants.GlobalConfig, $""" |
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.
Not sure that this needs to be constant since we only use it once.
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.
We might want to write different analyzers later on, tho.
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 think we should only add it when we need it then. But it's also not a big deal, we can keep it.
...ot.NET.Sdk/Godot.SourceGenerators.Tests/GeneratorsTests/ScriptPathAttributeGeneratorTests.cs
Outdated
Show resolved
Hide resolved
ea9f8fc
to
1797d98
Compare
@raulsntos the style check is inconsistent with the guidelines:
Should we change the guidelines, or the check? I'm generally more inclined to add spaces, but removed them in my last commit after recently re-reading the doc page. |
We seem to have moved away from that guideline, probably because by the default VSCode adds spaces. So I'd say we should update the guidelines. |
1797d98
to
a803774
Compare
11de997
to
ed98461
Compare
Brought this up to date, and removed everything not related to those new tests. |
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ScriptSignalsGeneratorTests.cs
Show resolved
Hide resolved
.../mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/ScriptPathAttributeGeneratorTests.cs
Show resolved
Hide resolved
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Extensions.cs
Outdated
Show resolved
Hide resolved
modules/mono/editor/Godot.NET.Sdk/Godot.SourceGenerators.Tests/Constants.cs
Outdated
Show resolved
Hide resolved
ed98461
to
8673157
Compare
8673157
to
e41feb3
Compare
Just rebased on dev. Would be nice to have that one merged now that we're out of the 4.3 rush 🙏 |
ScriptPathAttributeGenerator