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

HYDRA-861 & HYDRA-932 : Fix MtoA & LookdevX loading in tests + Related test improvements #108

Merged
merged 19 commits into from
Mar 26, 2024

Conversation

debloip-adsk
Copy link
Collaborator

@debloip-adsk debloip-adsk commented Mar 25, 2024

This PR contains the following changes :

  • Fixed MtoA and LookdevX loading
  • Reworked plugin loading in tests to make it so failing to load a plugin fails the test instead of skipping it. This is to avoid the MtoA/LookdevX situation where the tests weren't working yet reported as passing.
  • Merged MtohTestCase into MayaHydraBaseTestCase, as the distinction was no longer really useful and was leading to code duplication
  • Converted absolute paths in test scenes to relative paths
  • Converted .usd files to .usda for readability

@debloip-adsk debloip-adsk changed the title HYDRA-861 & HYDRA-932 : Fix MtoA & LookdevX loading in tests + Use relative paths in test scenes HYDRA-861 & HYDRA-932 : Fix MtoA & LookdevX loading in tests + Related test framework improvements Mar 25, 2024
@debloip-adsk debloip-adsk force-pushed the debloip/HYDRA-861/HYDRA-932/fix-mtoa-loodkevx-tests branch from 776285a to 18bcfdf Compare March 25, 2024 20:16
@@ -26,7 +26,7 @@ For example, to run testVisibility on RelWithDebInfo :

### Running tests with MayaUSD

Some tests require the MayaUSD plugin to be loaded in order to be run. If MayaUSD cannot be loaded, those tests will be skipped. In order for the test framework to find and load MayaUSD, the `-DMAYAUSD_LOCATION` CMake flag must be set to your MayaUSD installation directory when building MayaHydra. For convenience, you can use the `--mayausd-location` parameter when using the [build.py](../../../../../build.py) script, which will set the CMake flag for you with the given argument. The specified path should point to the directory where your MayaUSD `.mod` file resides. Note that the test framework will not find MayaUSD using the MAYA_MODULE_PATH environment variable.
The tests require MayaUSD in order to be run. In order for the test framework to find and load MayaUSD, the `-DMAYAUSD_LOCATION` CMake flag must be set to your MayaUSD installation directory when building MayaHydra. For convenience, you can use the `--mayausd-location` parameter when using the [build.py](../../../../../build.py) script, which will set the CMake flag for you with the given argument. The specified path should point to the directory where your MayaUSD `.mod` file resides. Note that the test framework will not find MayaUSD using the MAYA_MODULE_PATH environment variable.
Copy link
Collaborator Author

@debloip-adsk debloip-adsk Mar 25, 2024

Choose a reason for hiding this comment

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

(see mtohUtils.py for the corresponding code changes) I made it so we load MayaUSD by default for all tests, since we expect MayaHydra to be built with it anyways.

4. Add the following snippet at the bottom of your file :
2. Create a test class that derives from `mtohUtils.MayaHydraBaseTestCase`.
3. Add the line `_file = __file__` in your class definition.
4. If needed, set `_extraPluginsToLoad` to list any plugins that need to be loaded for your test. MayaUSD does not need to be specified, it will be loaded automatically.
Copy link
Collaborator Author

@debloip-adsk debloip-adsk Mar 25, 2024

Choose a reason for hiding this comment

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

(see mtohUtils.py for the corresponding code changes) The _extraPluginsToLoad variable will be used to list any plugins required by the test suite. As opposed to the previous method of skipping a test if it failed to load, this method will try to load the listed plugins, but fail the test suite immediately if one of them fails to load.

Comment on lines -67 to +69
- By default, the mayaHydra plugin will not be loaded. An easy way to load it is to have your test class derive from `MayaHydraBaseTestCase`, which will automatically load mayaHydra when running tests. Don't forget to add the line `_file = __file__` in your class.
- After loading mayaHydra, the renderer will still be on Viewport 2.0. If you want to use another renderer, your test will have to switch to it. If you want to use HdStorm, an easy way to do it is to have your test class derive from `MayaHydraBaseTestCase` and call `self.setHdStormRenderer()`
- Before each test, a new file will be opened and the renderer will be switched to Hydra. If you need to switch between renderers, you can use the `self.setHdStormRenderer()` and `self.setViewport2Renderer()` methods.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed most of the previous info as it was outdated, and removed the mention of "MayaHydra not being loaded by default unless deriving from MayaHydraBaseTestCase", since rarely (if ever) a test will want to derive from unittest.TestCase directly. I've also adjusted the steps to reflect that.

Comment on lines -75 to +79
1. Make your test class derive from `MtohTestCase`.
2. Don't forget to add the line `_file = __file__` in your class.
3. Create a folder called [TestName]Test (e.g. for testVisibility, create a folder VisibilityTest)
4. Put the images you want to use for comparison in that folder
5. Call `self.assertImagesClose`, `self.assertImagesEqual` and/or `self.assertSnapshotClose` with the file names of the images to compare with.
1. Make sure your test class derives from `mtohUtils.MayaHydraBaseTestCase` (and has the line `_file = __file__`).
Copy link
Collaborator Author

@debloip-adsk debloip-adsk Mar 25, 2024

Choose a reason for hiding this comment

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

See mtohUtils.py for the corresponding code changes and explanation of merging MtohTestCase into MayaHydraBaseTestCase

Comment on lines -91 to +94
Utility files are located under [/test/testUtils](../../../../testUtils/). Note that at the time of writing this (2023/08/16), many of the utils files and their contents were inherited from the USD plugin, and are not all used. Here are some of the main utilities you might find useful :

- mtohutils.py :
- `checkForMayaUsdPlugin()` will try to load the MayaUsd plugin and return a boolean indicating if the plugin was loaded successfully. By decorating a test case with the following line, you can run the test only if the MayaUsd plugin is found and loaded :
```@unittest.skipUnless(mtohUtils.checkForMayaUsdPlugin(), "Requires Maya USD Plugin.")```
- `MayaHydraBaseTestCase` is a base class you can use for your test classes that will provide you with some useful functionality, such as automatically loading the mayaHydra plugin. It also provides a method to use the HdStorm renderer (`setHdStormRenderer`), and another to create a simple scene with a cube (`makeCubeScene`).
- `MtohTestCase` is a class you can use as a base for your test classes that will provide you with image comparison functionality (see [Image comparison](#Image-comparison) section). Note that this class already derives from MayaHydraBaseTestCase, so it will also automatically load the mayaHydra plugin.
Utility files are located under [/test/testUtils](../../../../testUtils/). Note that at the time of writing this (2023/08/16), many of the utils files and their contents were inherited from the USD plugin, and are not all used.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed most info as it is outdated or not very relevant. The plugin loading info has been moved into the steps to add tests + the best practices section

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Most differences here are merging the MtohTestCase-specific code into MayaHydraBaseTestCase. Historically, the MayaHydraBaseTestCase class was a read-only class for tests that didn't output anything, and MtohTestCase was the image-diffing subclass that set up an output directory. However, since code coverage was added, MayaHydraBaseTestCase now also sets up an output directory, which is kind of duplicated from MtohTestCase. The plugin loading rework code would also have needed to be duplicated in both setupClass methods of MayaHydraBaseTestCase and MtohTestCase, so it felt simpler to just merge the two. Additionally, the data-source dumping setup that should be coming soon will also need pretty much the same setup as MtohTestCase, so I feel like there is not much point to keeping separate classes, and just merging them into one will be easier to maintain.

_file = None

DEFAULT_CAM_DIST = 24
_extraPluginsToLoad = []
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New variable to list what plugins a test suite needs

Comment on lines 72 to 79
if MAYAUSD_PLUGIN_NAME not in cls._extraPluginsToLoad:
cls._extraPluginsToLoad.append(MAYAUSD_PLUGIN_NAME)

for pluginToLoad in cls._extraPluginsToLoad:
# If a plugin fails to load, the entire test suite will be immediately aborted.
# Note that in the case of mtoa, the plugin might load successfully but not
# initialize properly, which means issues will only be caught in the actual tests.
cmds.loadPlugin(pluginToLoad)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Plugin-loading code that will fail the test suite if one fails to load (with one asterisk, see the comment). We add MayaUSD to the list if it is not specified, since we expect MayaHydra to have been built with it. This allows most tests to not have to define _extraPluginsToLoad if all they need is MayaUSD, which will be quite common.

@debloip-adsk debloip-adsk changed the title HYDRA-861 & HYDRA-932 : Fix MtoA & LookdevX loading in tests + Related test framework improvements HYDRA-861 & HYDRA-932 : Fix MtoA & LookdevX loading in tests + Related test improvements Mar 25, 2024
@debloip-adsk debloip-adsk marked this pull request as ready for review March 25, 2024 21:14
@debloip-adsk debloip-adsk self-assigned this Mar 25, 2024
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed Arnold references as per HYDRA-932. Also removed the USD stage references as there is no associated stage.

@debloip-adsk
Copy link
Collaborator Author

3. Create a folder called [TestName]Test (e.g. for testVisibility, create a folder VisibilityTest)
4. Put the images you want to use for comparison in that folder
5. Call `self.assertImagesClose`, `self.assertImagesEqual` and/or `self.assertSnapshotClose` with the file names of the images to compare with.
1. Make sure your test class derives from `mtohUtils.MayaHydraBaseTestCase` (and has the line `_file = __file__`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

side question: is it possible to avoid the copying of "_file = file" each time?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So, this is used in order to set the _inputDir variable, which points to the input folder containing the reference images (or dumped data sources), by using the given file path and test class name. Since the input folders live in the same hierarchy as the test files, __file__ is used to get the path to test file, which can then be used to construct the path to its corresponding input folder. I've tried other means of obtaining the path to the test file/class from the generic test setup code, such as by using inspect.getfile(), but it does not seem to work. With the relative paths between the test setup code and test files being known, we could in theory use these to construct the path to the input folders, but that would need to be changed along with any directory structure change, and we'd also need to account for C++ tests being in a subdirectory. At this point I think it'd be more trouble than it's worth, so I suggest staying with what we have.

cls._testDir = os.path.abspath('.')

if MAYAUSD_PLUGIN_NAME not in cls._extraPluginsToLoad:
cls._extraPluginsToLoad.append(MAYAUSD_PLUGIN_NAME)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it better to set _extraPluginsToLoad = [MAYAUSD_PLUGIN_NAME] in MayaHydraBaseTestCase, so the derived test case has an opportunity to not load mayaUSDPlugin if needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Following our discussion in the scrum, I suggest leaving it as is currently, and adjust later if needed.

Copy link
Collaborator

@ppt-adsk ppt-adsk left a comment

Choose a reason for hiding this comment

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

Great work, thanks for the cleanup!

test/testUtils/mtohUtils.py Outdated Show resolved Hide resolved
lilike-adsk
lilike-adsk previously approved these changes Mar 26, 2024
Copy link
Collaborator

@lilike-adsk lilike-adsk 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!

ppt-adsk
ppt-adsk previously approved these changes Mar 26, 2024
@debloip-adsk debloip-adsk dismissed stale reviews from ppt-adsk and lilike-adsk via 31354e8 March 26, 2024 19:44
@debloip-adsk debloip-adsk added ready-for-merge Development process is finished, PR is ready for merge test labels Mar 26, 2024
@lilike-adsk lilike-adsk merged commit b5e22e1 into dev Mar 26, 2024
10 checks passed
@lilike-adsk lilike-adsk deleted the debloip/HYDRA-861/HYDRA-932/fix-mtoa-loodkevx-tests branch March 26, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge Development process is finished, PR is ready for merge test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants