Make import / export tests run from core. #595

ppt-adsk · 2020-06-18T13:58:09Z

This pull request moves the majority of import / export tests down to the maya-usd core, and has them run using the mayaUsdPlugin. Only those tests that have a dependence on the Pixar plugin (for Maya scene assembly support) were kept in the Pixar plugin subtree.

ppt-adsk · 2020-06-18T14:10:37Z

plugin/pxr/maya/lib/usdMaya/testenv/UsdExportPointInstancerTest/InstancerTest.ma

@@ -0,0 +1,262 @@
+//Maya ASCII 2016 scene


This file was actually removed by
7cd3c48
Will fix.

ppt-adsk · 2020-06-18T14:15:17Z

plugin/pxr/maya/lib/usdMaya/testenv/testUsdExportPointInstancer.py

-        cmds.loadPlugin("MASH")
-
-        scene = "InstancerTestMash.ma"
+        # Choose the test file based on whether the MASH plugin is available.


Again, incorrect revert of
7cd3c48
will fix.

ppt-adsk · 2020-06-18T14:29:54Z

test/lib/usd/importExport/testUsdExportStripNamespaces.py

@@ -49,8 +48,22 @@ def testExportWithClashStripping(self):

        usdFilePath = os.path.abspath('UsdExportStripNamespaces_EXPORTED.usda')

-        errorRegexp = "Multiple dag nodes map to the same prim path" \
-            ".+|cube1 - |foo:cube1.*"
+        # https://github.com/Autodesk/maya-usd/blob/dev/lib/mayaUsd/fileio/jobs/writeJob.cpp#L814


Help with this would be appreciated.

ppt-adsk · 2020-06-18T14:33:33Z

Many, many thanks to @HamedSabri-adsk for his help on this branch!

HamedSabri-adsk · 2020-06-18T15:09:19Z

test/lib/usd/importExport/fixturesUtils.py

+    '''
+    (testDir, testFile) = _setUpClass(modulePathName)
+
+    return testDir


@ppt-adsk This is a great utility. I am wondering if it makes sense to have fixturesUtils.py move out of here to a higher directory ( e.g test\lib\usd ). In case we bring in more pixar tests here, they can also leverage this.

HamedSabri-adsk · 2020-06-18T15:14:49Z

test/lib/usd/importExport/CMakeLists.txt

+        PYTHON_MODULE ${target}
+        WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+        ENV
+            "MAYA_DISABLE_CIP=1"


I believe we can set below two Maya environment variables in test.cmake, so don't have to worry about setting them every time.

MAYA_DISABLE_CIP=1 Avoid fatal crash on start-up. MAYA_DISABLE_CER=1 Customer Error Reporting.

These can be added to the list of ENV variables like this:

# without "MAYA_NO_STANDALONE_ATEXIT=1", standalone.uninitialize() will # set exitcode to 0 set_property(TEST "${test_name}" APPEND PROPERTY ENVIRONMENT "MAYA_NO_STANDALONE_ATEXIT=1" "MAYA_DISABLE_CIP=1" "MAYA_DISABLE_CER=1")

HamedSabri-adsk · 2020-06-18T15:20:25Z

test/lib/usd/importExport/CMakeLists.txt

+    testUsdExportMesh.py
+    testUsdExportNurbsCurve.py
+    testUsdExportOpenLayer.py
+    testUsdExportOverImport.py


Hey @ppt-adsk ,

I did a clean build of maya-usd with your changes on Windows. Ran all the tests and ctest reported that all tests pass:

100% tests passed, 0 tests failed out of 104 Label Time Summary: importExport = 667.42 sec*proc (39 tests) ufe = 390.75 sec*proc (19 tests) usdPreviewSurface = 33.33 sec*proc (2 tests) Total Test time (real) = 85.62 sec

I then ran the tests again but ctest reports that testUsdExportOverImport failed. Do you see this behavior on your side as well?

Here is what -VV shows:

40: ====================================================================== 40: FAIL: testImportModifyAndExportCubeModel (testUsdExportOverImport.testUsdExportOverImport) 40: ---------------------------------------------------------------------- 40: Traceback (most recent call last): 40: File "testUsdExportOverImport.py", line 73, in testImportModifyAndExportCubeModel 40: File "testUsdExportOverImport.py", line 50, in _ValidateUsdBeforeExport 40: AssertionError: invalid null prim is not true 40: 40: ---------------------------------------------------------------------- 40: Ran 1 test in 0.323s 40: 40: FAILED (failures=1) 1/1 Test #40: testUsdExportOverImport ..........***Failed 6.35 sec

Great catch! I do get the same failure. Let me investigate.

HamedSabri-adsk · 2020-06-18T15:26:09Z

@ppt-adsk Great job. You also did a lot of the work here as well.

Aside from the testUsdExportOverImport failure and couple of minor comments, everything else looks good to me.

Merci

mattyjams · 2020-06-18T23:20:33Z

Thanks for tackling this @ppt-adsk!

I unfortunately probably won't get a chance to dig into this in detail until early next week, but I'm excited to check it out!

ppt-adsk · 2020-06-22T13:14:51Z

@mattyjams no worries, thanks for having a look.

ppt-adsk · 2020-06-22T13:16:17Z

importExport directory renamed translators at @kxl-adsk 's request, to match library hierarchy and help discoverability.

mattyjams

Looks good to me, thanks @ppt-adsk!

Just one request: Can we make the fixturesUtils changes to the testUsdExportRfMLight.py test? I don't see any modifications to that file. No need to make sure it actually runs and passes. I can verify that it's working when I merge this internally.

And on that note, fair warning that I may have some small follow-ups to this once I'm able to wrestle our internal build and test system to adapt to these changes. One thing I think might be problematic for us is that we currently install the test scripts in a different location than the test run directory, so I may need to make fixturesUtils.setUpClass() slightly more flexible to be able to handle that somehow.

mattyjams · 2020-06-24T17:26:58Z

test/lib/usd/translators/fixturesUtils.py

+        shutil.rmtree(outputPath)
+
+    os.mkdir(outputPath)
+    os.chdir(outputPath)


Curious about this, since by the time we hit this we've obviously already launched Maya from some other directory. Would Maya itself possibly still write workspace.mel or swatch files or other stuff into the run directory, and do we care if so?

@JGamache-autodesk has suggested the following improvements, which I believe touch upon what you're mentioning:

MAYA_APP_DIR -> should point to somewhere inside that folder. Makes sure tests are always run with fresh user prefs, and allows investigating prefs added or modified by the test.
MAYA_PROJECTS_DIR -> should point to %{ MAYA_APP_DIR}/projects, especially if the test renders to disk using the default output settings or exports a USD scene to the default output path by only specifying a file name.

This is for later improvement, as needed.

test/lib/usd/translators/fixturesUtils.py

mattyjams · 2020-06-24T17:43:22Z

test/lib/usd/translators/testUsdExportStripNamespaces.py

+        # Unfortunately, at time of writing, this is not happening correctly,
+        # as the Python exception is generated, but the detailed error string
+        # is lost.  We are left with the the generic 'Maya command error'
+        # string, which is far weaker.  PPT, 16-Jun-20.


Hmm, interesting. This test passes currently with the original regex, so was the problem caused by the file moves somehow?

Is there an issue open for this, or can we create one if not?

Excellent suggestion:
#601

mattyjams · 2020-06-24T17:47:23Z

test/lib/usd/translators/testUsdImportRfMLight.py

-        cmds.loadPlugin('pxrUsd')
-        cmds.usdImport(file=usdFilePath, shadingMode='pxrRis',
-            readAnimData=True)
+        usdFilePath = os.path.join(os.path.dirname(os.path.realpath(__file__)), "UsdImportRfMLightTest", "RfMLightsTest.usda")


Not using fixtureUtils for this?

ppt-adsk · 2020-06-24T20:49:56Z

Looks good to me, thanks @ppt-adsk!

Thanks @mattyjams for the review!

Just one request: Can we make the fixturesUtils changes to the testUsdExportRfMLight.py test? I don't see any modifications to that file. No need to make sure it actually runs and passes. I can verify that it's working when I merge this internally.

Done, see
5d1f558

And on that note, fair warning that I may have some small follow-ups to this once I'm able to wrestle our internal build and test system to adapt to these changes. One thing I think might be problematic for us is that we currently install the test scripts in a different location than the test run directory, so I may need to make fixturesUtils.setUpClass() slightly more flexible to be able to handle that somehow.

That sounds like a great improvement.

mattyjams · 2020-06-24T22:53:05Z

Just one request: Can we make the fixturesUtils changes to the testUsdExportRfMLight.py test? I don't see any modifications to that file. No need to make sure it actually runs and passes. I can verify that it's working when I merge this internally.

Done, see
5d1f558

Thanks @ppt-adsk! I think 5d1f558 covers testUsdImportRfMLight.py, but testUsdExportRfMLight.py needs the same treatment? It looks like it's currently unchanged.

ppt-adsk · 2020-06-25T00:47:22Z

Thanks @ppt-adsk! I think 5d1f558 covers testUsdImportRfMLight.py, but testUsdExportRfMLight.py needs the same treatment? It looks like it's currently unchanged.

Ah, sorry, missed that one. Done in
51bee04

Make import / export tests run from core.

ab977bd

kxl-adsk assigned ecp-maya-devops-adsk Jun 18, 2020

kxl-adsk requested review from mattyjams and HamedSabri-adsk June 18, 2020 14:10

kxl-adsk added import-export Related to Import and/or Export unit test Related to unit tests (both python or c++) labels Jun 18, 2020

ppt-adsk commented Jun 18, 2020

View reviewed changes

HamedSabri-adsk reviewed Jun 18, 2020

View reviewed changes

Addressed ADSK code review comments.

ce026b8

fowlertADSK mentioned this pull request Jun 22, 2020

MAYA-103187 Add option to save .usd files as binary or ascii. #591

Closed

mattyjams approved these changes Jun 24, 2020

View reviewed changes

ppt-adsk mentioned this pull request Jun 24, 2020

testUsdExportStripNamespaces test TF_RUNTIME_ERROR to Python exception detailed error string lost #601

Open

Addressed Pixar code review comments.

5d1f558

ppt-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jun 24, 2020

Converted testUsdExportRfMLight to use fixturesUtils.

51bee04

ppt-adsk assigned ecp-maya-devops-adsk and unassigned ecp-maya-devops-adsk Jun 25, 2020

kxl-adsk merged commit 7d6d196 into dev Jun 25, 2020

kxl-adsk deleted the tremblp/MAYA-104105/import_export_tests branch June 25, 2020 10:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make import / export tests run from core. #595

Make import / export tests run from core. #595

ppt-adsk commented Jun 18, 2020 •

edited

Loading

ppt-adsk Jun 18, 2020

ppt-adsk Jun 18, 2020

ppt-adsk Jun 18, 2020

ppt-adsk commented Jun 18, 2020

HamedSabri-adsk Jun 18, 2020

HamedSabri-adsk Jun 18, 2020 •

edited

Loading

HamedSabri-adsk Jun 18, 2020

ppt-adsk Jun 18, 2020

HamedSabri-adsk commented Jun 18, 2020

mattyjams commented Jun 18, 2020

ppt-adsk commented Jun 22, 2020

ppt-adsk commented Jun 22, 2020

mattyjams left a comment

mattyjams Jun 24, 2020

ppt-adsk Jun 24, 2020

mattyjams Jun 24, 2020

ppt-adsk Jun 24, 2020

mattyjams Jun 24, 2020

ppt-adsk commented Jun 24, 2020

mattyjams commented Jun 24, 2020

ppt-adsk commented Jun 25, 2020

Make import / export tests run from core. #595

Make import / export tests run from core. #595

Conversation

ppt-adsk commented Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppt-adsk commented Jun 18, 2020

Choose a reason for hiding this comment

HamedSabri-adsk Jun 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HamedSabri-adsk commented Jun 18, 2020

mattyjams commented Jun 18, 2020

ppt-adsk commented Jun 22, 2020

ppt-adsk commented Jun 22, 2020

mattyjams left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ppt-adsk commented Jun 24, 2020

mattyjams commented Jun 24, 2020

ppt-adsk commented Jun 25, 2020

ppt-adsk commented Jun 18, 2020 •

edited

Loading

HamedSabri-adsk Jun 18, 2020 •

edited

Loading