Skip to content
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

EMSUSD-820 run more of the Pixar plugin tests #3479

Merged
merged 7 commits into from
Dec 13, 2023

Conversation

pierrebai-adsk
Copy link
Collaborator

@pierrebai-adsk pierrebai-adsk commented Nov 22, 2023

There were tests in the "pxrUsdMayaGL" folder in the pxr (Pixar) plugin that
were not run. Make these tests be run when testing the plugin in the automated
builds using Cmake as the build system.

  • Add a CMakeLists.txt to setup the test to be run when built using cmake.
  • Make sure the folder paths are correct while running the test so that the images are generated where they are expected to compare them to the baseline images.
  • Make sure the relative USD files used in assembly are found by temporarily changing the current directory during the test.
  • Add one missing baseline image file.

There were tests in the "pxrUsdMayaGL" folder in the pxr (Pixar) plugin that
were not run. As a first step, make these test be run when testing the plugin.

- Add a CMakeLists.txt to setup the test to be run when built using cmake.
- Make sure the folder paths are correct while running the test.
- Make sure the relative USD files used in assembly are found.
- Add one missing baseline image file.
- Disable the testRefAssemblyDrawRepresentations unit test since it fails in the "Expanded" mode.
@pierrebai-adsk pierrebai-adsk added pxr Related to Pixar plugin unit test Related to unit tests (both python or c++) labels Nov 22, 2023
@dj-mcg
Copy link
Collaborator

dj-mcg commented Nov 22, 2023

Is the plan to leave the test disabled until we find the issue on our end? We actually have hacked out the LoadNone change and disabled the testProxyShapeBase test since this issue directly impacts our productions.

Thanks.

@pierrebai-adsk
Copy link
Collaborator Author

Yes. It was unclear at this point if you were still investigating the source of the problem and found a solution or not. In your builds, reverting my changes fixes the problem? That is strange since on our side, building with MayaUSD versions older than my changes still fails to have the "Expanded" mode of the assembly be drawn red.

@pierrebai-adsk pierrebai-adsk changed the title EMSUSD-820 run more of the Pxiar plugin tests EMSUSD-820 run more of the Pixar plugin tests Nov 24, 2023
There were tests in the "pxrUsdMayaGL" folder in the pxr (Pixar) plugin that
were not run. Make these tests be run when testing the plugin in the automated
builds using Cmake as the build system.

- Add a CMakeLists.txt to setup the test to be run when built using cmake.
- Make sure the folder paths are correct while running the test so that the
  images are generated where they are expected to compare them to the baseline
  images.
- Make sure the relative USD files used in assembly are found by temporarily
  changing the current directory during the test.
- Add one missing baseline image file.
…sk/maya-usd into bailp/EMSUSD-820/more-pixar-tests
Different Maya version have different defaults for lightning. Add different baseline images to compensate the difference when testing in older Maya versions.
@pierrebai-adsk
Copy link
Collaborator Author

pierrebai-adsk commented Dec 8, 2023

@dj-mcg putting you as reviewer to verify if the changes made to the test and the additional images would be a problem in Pixar's internal build system and testing suite.

Remove the now unecessary additional image.
dj-mcg
dj-mcg previously approved these changes Dec 11, 2023
Copy link
Collaborator

@dj-mcg dj-mcg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for all the work!

@pierrebai-adsk pierrebai-adsk added the ready-for-merge Development process is finished, PR is ready for merge label Dec 12, 2023
@seando-adsk seando-adsk merged commit 0528a4b into dev Dec 13, 2023
@seando-adsk seando-adsk deleted the bailp/EMSUSD-820/more-pixar-tests branch December 13, 2023 00:24
@@ -70,25 +121,30 @@ def testGenerateImages(self):
cmds.file(os.path.abspath('InstancerDrawTest.ma'),
open=True, force=True)

# The cards rendering colors in older versions of Maya is lighter,
suffix = ''
if mayaUtils.mayaMajorVersion() <= 2024:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pierrebai-adsk - I think we actually need to be checking the MayaUsd version here, since this was a change I pushed back in 10/22, and as a result I'm picking up this imaging difference even though I'm still on Maya 2023. What's the best way to do that from test python scripts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming cmds.pluginInfo('mayaUsdPlugin', q=True, v=True) would be correct approach?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PS. Here is the change that introduced the behavior:
#2680

If I'm following versioning history correctly, that means we will have the new imaging results in mayaUsd v0.23.0 and beyond, is that correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, just change the test in the backported version. The test is within MayaUSD itself, so it would be strange to check the MayaUSD version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pxr Related to Pixar plugin ready-for-merge Development process is finished, PR is ready for merge unit test Related to unit tests (both python or c++)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants