-
Notifications
You must be signed in to change notification settings - Fork 714
Fixes #4102. Add AOT version of Tests #4134
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
base: v2_develop
Are you sure you want to change the base?
Conversation
Impressive work @BDisp! I'm kinda blown away they pass. |
…l.Gui into BDisp-v2_4102_add-aot-unit-tests
I'm getting errors trying to run
|
Tweaks to FactSkipIfAOTAttribute
Merged. Thanks. |
…: 'Index expressions are only supported with constants.'
The commit d75456e allow to link the |
I know I originally suggested that all the Examples be set to compile for AOT as well as the tests. But if we have to add a ton of complexity to the samples for that then I have changed my mind. Just making UI Catalog do it is more than enough. It's ok for UI Catalog to have extra complexity because it IS a test as well as sample. But the others are primarily SAMPLES and should not be cluttered with things that don't let them illustrate just the bare minimum. I think this is how we think about each project in ./Examples:
I really appreciate you putting the effort in and I feel bad that you did a bunch of work that's not needed. But we need the samples to be as simple as possible to ensure new developers to the project have as simple an experience as possible. |
Due this work I found that some things was not working well. The user doesn't not have to do any of this complexity things because the linked and copied files are available as if they are part of each project. I consider important to see how the different examples projects behave in each OS and in each driver, which with only the |
One think I discover with this PR is the |
Seems like a bug for sure! I totally appreciate that playing with these samples found bugs. However, the right way to deal with those found bugs is to build a unit test that reproduces it. Adding complexity to samples that are intended to be simple is not the right way. |
Do you want I close this then? |
No, why? I'm just asking you not to complexify the samples as part of this. |
The part of the |
I say remove it before we fully merge this. If it helps temporarily great. |
I'm also getting this error. I think |
Fixes
Proposed Changes/Todos
UnitTests.AOT
projectMoq
packages due to not be AOT compatible. Our concern is only to makeTerminal.Gui
AOT compatiblePull Request checklist:
CTRL-K-D
to automatically reformat your files before committing.dotnet test
before commit///
style comments)