stride3d
Test case improvements and updates
PR Details
Goal was to try and go through failing or skipped test cases and provide some general maintenence and fixing tests that constaintly fail. I feel like for some of these, there may be some discussion about if they should be skipped or not if they always fail.
Overall in this PR, I got 7 Tests to now pass that could not pass before (or where skipped).
4 in TypeConverter Half tests
- A large part of these tests failing was a result of improper ToString implementation and incomplete functionality.
- Was able to add the proper ToString integration to Half2, Half3 and Half4 converters. Was trying to follow the comments to match against other converters as best as possible.
- Added TODO comments for further verification
LifetimeNoSimpleConstructor
- This test was failing due to a missing empty constuctor causing problems in serialization. I added the empty constructor and still kept the test case the same. The test case itself is a bit wierd so would want to get a confirmation that getting it to pass with this change still keeps it a valid test.
TestGCHandleAlloc
- Looking through the test its unclear why the Autolayout test was ever expected to return false. This might be question to pass off to devs who review this.
TestGetFontInfo
- This values being testing always has inconsistent results, the goal of the test case was to see if the test font can be properly loaded but fails because of fluctuating values like baseline, width, and height having different values (these are dependent on how a new UnitsPerEM is choosen each time the test is ran). I changed the test to instead of checking hardcoded numbers to check if they are initially changed from their starting value of zero. In the case of lineSpacing, the result is always 0. I left the original values for the old tests but can remove them if needed to clean up.
Added 41 Skips for tests that will always seem to fail. These are all rendering related. Due to the fact that GPU rendering is not deterministic, devs would need a NVIDIA GeForce GTX 960 on their own personal workspace for these tests to pass. However these will be used in the future when the teamcity agent is back up and running and these tests will be no longer skipped. The images will also most likely outdated. Reviews of this PR might indicate that these skips should be removed and instead a note for others that they should fail if the proper hardward isnt used. Note: There was a push that was related to updating the rendering test images, that was reverted and is not in this PR but reviewers may see references to a push that involved updating images.
1 Improvement to a test suite: Fix related to change in StrideDefaultAssetPlugin I was able to fix an issue that was occuring all the tests in the Stride.Samples.Tests suite. Originally I was seeing that the tests could not find certain directories which turned out to be case sensitive. After this change, the tests can now get farther but there is now a new error they hit which appears to be related to dotnet package issues. I think the scope of this test case failing exceeded what I was originally set out to do so for now I'll include this change for now. However I may need confirmation since I'm not sure if this current test suite failure is a result of my current workspace being set up incorrectly or not.
Related Issue
N/A
Types of changes
- [ ] Docs change / refactoring / dependency upgrade
- [X] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
Checklist
- [ ] My change requires a change to the documentation.
- [ ] I have added tests to cover my changes. (technically didnt add any NEW tests)
- [X] All new and existing tests passed. (It seems that the only area where I get super clear and consistent failures is Stride.Samples.Tests - about 32 test failures remaining)
- [X] I have built and run the editor to try this change out.
Added 41 Skips for tests that will always seem to fail. These are all rendering related. Due to the fact that GPU rendering is not deterministic, devs would need a NVIDIA GeForce GTX 960 on their own personal workspace for these tests to pass. However these will be used in the future when the teamcity agent is back up and running and these tests will be no longer skipped. The images will also most likely outdated. Reviews of this PR might indicate that these skips should be removed and instead a note for others that they should fail if the proper hardward isnt used.
While this is true, you can check if the graphics tests pass locally using your own GPU.
When you run the tests, it will create in the tests directory a subdirectory with the generated images for your GPU. That directory will be named according to your GPU's name.
If you copy that directory besides the one named NVIDIA GeForce GTX 960, the next time you run the tests, they will discover your reference images and compare against those, thus passing the tests (or failing) if the results are consistent.
Tell me if you have any question and I'll try to reproduce it and explain it better.
Added 41 Skips for tests that will always seem to fail. These are all rendering related. Due to the fact that GPU rendering is not deterministic, devs would need a NVIDIA GeForce GTX 960 on their own personal workspace for these tests to pass. However these will be used in the future when the teamcity agent is back up and running and these tests will be no longer skipped. The images will also most likely outdated. Reviews of this PR might indicate that these skips should be removed and instead a note for others that they should fail if the proper hardward isnt used.
While this is true, you can check if the graphics tests pass locally using your own GPU.
When you run the tests, it will create in the
testsdirectory a subdirectory with the generated images for your GPU. That directory will be named according to your GPU's name.If you copy that directory besides the one named
NVIDIA GeForce GTX 960, the next time you run the tests, they will discover your reference images and compare against those, thus passing the tests (or failing) if the results are consistent.Tell me if you have any question and I'll try to reproduce it and explain it better.
Oh! Gotcha okay, so I guess with that kinda confirmed, I think my confusion about these were more so seeing only the GTX 960 directories listed in the project. So essentially the workflow would be to first test and save out your current GPU directory from the master branch and then copy those out into your branch you have your changes in? I think what confused me was no one else pushes their GPU directories. So ideally I should roll those changes back and remove the skips. And I shouldnt push the copied directory with the GPU tests also right? I guess if everyone did that we would have a ton of test directories. Thank you for the insight!
Also followup cause I am newer, should this info of this workflow of rendering tests be written down anywhere? And would this workflow change when the teamcity agent is back up?
So essentially the workflow would be to first test and save out your current GPU directory from the master branch and then copy those out into your branch you have your changes in?
The way I do it usually is the following:
-
First I run the tests. They will all fail because my GPU is different and there will be no reference images. But in the process, it will create one (or several, can't recall) directories named the same as your GPU (in my case it would be
AMD Radeon RX 6800XT). -
I take those newly created directories and move them beside the default ones named
NVIDIA GeForce GTX 960, each directory in the same category it is taken from (soStride.Engine.TeststoStride.Engine.Testsand so on). Sometimes I compare manually the generated images with the reference ones to check they are generating similar things. -
If all has gone well, the next time you run the tests it will recon your GPU as one of the ones it has reference images for, compare your results with those reference images and (if the images match) the tests will pass.
I however, would only do this locally when modifying the graphics or rendering parts of the engine, which would be the time where these things are useful to verify no regressions have happened.
Ideally, a complete set of reference images more current than those for the GTX 960 would be needed, featuring a more contemporary GPU that allows us to update and expand that set of reference images as new features and effects are added. However, as this is a community-driven project, it would require a contributor to provide an actual GPU for regular testing.
Also, I believe that the reference images for the GTX 960 include some things that are no longer part of Stride or are no longer testable. You'll notice the images generated for your GPU are way less than the currently existing ones.
Oh! Gotcha okay, so I guess with that kinda confirmed, I think my confusion about these were more so seeing only the GTX 960 directories listed in the project.
The GTX 960 directory has the reference images that are currently being validated in a TeamCity agent whenever a test run is initiated. I think Xen has a local PC with that nVidia GTX 960 exclusively for running these tests.
I think what confused me was no one else pushes their GPU directories.
I also don't think it would be a good idea to push other directories for other GPUs to GitHub because that would be many many different directories, each one full of big but very similar images, and it would only benefit the people with exactly those exact GPU configurations.
And I shouldnt push the copied directory with the GPU tests also right? I guess if everyone did that we would have a ton of test directories.
Exactly.
Should this info of this workflow of rendering tests be written down anywhere? And would this workflow change when the TeamCity agent is back up?
Yes. I would document it. Haven't done it yet because it is a not very useful process. Currently, it only serves to verify you don't break things locally using your own GPU.
Also, I believe that the reference images for the
GTX 960include some things that are no longer part of Stride or are no longer testable. You'll notice the images generated for your GPU are way less than the currently existing ones.
Awesome! Ok cool, yes that cleared up everything. Thanks for the fast reply! Would it be a good idea to remove those extra images you had mentioned? I did notice that as I was looking at this.
Yes. I would document it. Haven't done it yet because it is a not very useful process. Currently, it only serves to verify you don't break things locally using your own GPU.
What would be your recommendation for how to approach documenting this? Would simply adding comments suffice or would documentation somewhere need to be updated?
Once again, thanks for the insight and help!
Would it be a good idea to remove those extra images you had mentioned? I did notice that as I was looking at this.
I'd say yes. But better to ask someone that knows better where those images came from and if it's a good idea to delete them. Maybe @xen2 or @Kryptos-FR
What would be your recommendation for how to approach documenting this? Would simply adding comments suffice or would documentation somewhere need to be updated?
As having a good test suite is good for the project, I'd say we should document it properly. If this changes in the future, we'll see, but for now we should aim to shed light on everyone of the dark corners of the Stride project (there are a few). Right now I think the best you can ask for all documentation things is @VaclavElias. He has done a great job with the official docs.
Once again, thanks for the insight and help!
Always a pleasure to help!
