Unity Build running for PRs (and Tests) #532
Conversation
mhochk
commented
Dec 16, 2019
mhochk
commented
- Updated the build definition to build the Unity code and run the tests, but not produce or publish a unity package (which requires a Pro/Plus license).
- Moving the Unity tests in the direction of comparing against the images already maintained with the models. Could not completely switch to this yet though due to some discovered render issues to be addressed in independent changes.
- Fixed a race condition in loading multiple images in a single model by reworking the ILoader interface.
- Unified the test scenes/prefabs so that the Main scene fully leverages the TestScenePrefab and the TestScene is no longer relevant.
|
You shouldn't check in all of the ACTUAL and EXPECTED images from the tests. #Resolved |
| m_IsPrefabParent: 0 | ||
| --- !u!114 &10002067 stripped | ||
| MonoBehaviour: |
There was a problem hiding this comment.
What are all of these changes in practice? #Resolved
There was a problem hiding this comment.
Instead of a custom light, camera, and GLTF component, using the TestScenePrefab that already contains all 3. The Main scene essentially becomes that prefab used by the test, plus a floor and the parts to easily pick which model you want loaded.
In reply to: 358474507 [](ancestors = 358474507)
|
And I would love to get rid of them both, but the changes needed to do that are getting larger than I want to try including in this change. The plan is to get rid of the EXPECTED images in favor of the ones already included with the models, but there is at least 1 shader bug that still needs to get tracked down and fixed before that can happen. At that point we can debate out if the ACTUALs should be checked in, or just compared at runtime. (I'm in favor of the later, but not going to fight much if there are differing opinions). In reply to: 566251733 [](ancestors = 566251733) |
| @@ -524,9 +538,10 @@ protected IEnumerator WaitUntilEnum(WaitUntil waitUntil) | |||
| private async Task LoadJson(string jsonFilePath) | |||
| { | |||
| #if !WINDOWS_UWP | |||
| if (IsMultithreaded && _options.ExternalDataLoader.HasSyncLoadMethod) | |||
| var dataLoader2 = _options.DataLoader as IDataLoader2; | |||
| if (IsMultithreaded && dataLoader2 != null) | |||
There was a problem hiding this comment.
dataLoader2 != null [](start = 26, length = 19)
If this targets a new enough version of .NET, you can use this syntax:
_options.DataLoader is IDataLoader2 dataLoader2 #Resolved
There was a problem hiding this comment.
| @@ -535,10 +550,9 @@ private async Task LoadJson(string jsonFilePath) | |||
| #endif | |||
| { | |||
| // HACK: Force the coroutine to run synchronously in the editor | |||
| await _options.ExternalDataLoader.LoadStream(jsonFilePath); | |||
| _gltfStream.Stream = await _options.DataLoader.LoadStreamAsync(jsonFilePath); | |||
There was a problem hiding this comment.
Remove this line or update it to something else. #Resolved
| @@ -22,18 +22,16 @@ public class RootMergeComponent : MonoBehaviour | |||
| private async Task Start() | |||
| { | |||
| var fullPath0 = Application.streamingAssetsPath + Path.DirectorySeparatorChar + asset0Path; | |||
| ILoader loader0 = new FileLoader(URIHelper.GetDirectoryName(fullPath0)); | |||
| IDataLoader2 loader0 = new FileLoader(URIHelper.GetDirectoryName(fullPath0)); | |||
There was a problem hiding this comment.
IDataLoader2 [](start = 3, length = 12)
nit: I think both of these loaders can just be IDataLoader (not IDataLoader2) #Resolved
…ough it means none of the tests are actually running in the lab
* Updated the build definition to build the Unity code and run the tests, but not produce or publish a unity package (which requires a Pro/Plus license). Also does not include running the tests that rely on graphics (which is currently all of them). * Moving the Unity tests in the direction of comparing against the images already maintained with the models. Could not completely switch to this yet though due to some discovered render issues to be addressed in independent changes. * Fixed a race condition in loading multiple images in a single model by reworking the ILoader interface. * Unified the test scenes/prefabs so that the Main scene fully leverages the TestScenePrefab and the TestScene is no longer relevant.
