RFC: Extend EditorTest tools to MaterialEditor

[jromnoa]

Summary:

There are a set of tools inside C:\git\o3de\Tools\LyTestTools\ly_test_tools\o3de\editor_test.py that allow for parallel test runs that run in batched sets. The problem is, these tools only work for tests that utilize Editor but it could also support MaterialEditor. It currently cannot support other executables (such as Launcher executables).

This RFC proposal will discuss the viability of this and also a rough outline of how we would go about implementing it for O3DE. Once completed, the results of this RFC will be used to cut tasks and assign work to get it implemented.

What is the relevance of this feature?

Since the inception of the parallel & batch test run tools, there has been an increase in test speed without much sacrifice to test efficiency. This is because these tools utilize return codes to validate test results instead of log lines, which are often plagued by race conditions, file permission issues, and other technical problems whereas return codes are not.

This change will expand this option beyond just tests that utilize Editor and apply it to the MaterialEditor also.

Feature design description:

The feature will be the expansion of our existing parallel & batched test tools. Once implemented, it would allow the existing parallel & batched test runs for Editor.exe to be extended to MaterialEditor test runs as well, utilizing this return code approach to verifying tests. We will save time on test runs while also gaining more test efficiency, with no real sacrifice to our existing approach to automated tests.

Technical design description:

It will work the same as Editor tests that utilize this functionality. There is a "test launcher" file that contains all of the tests to batch, run in parallel, or run as a single non-batched non-parallel test. An example launcher file would look something like this:

# Full example can be found at C:\git\o3de\AutomatedTesting\Gem\PythonTests\Atom\TestSuite_Main.py

from ly_test_tools.o3de.editor_test import EditorSharedTest, EditorTestSuite

logger = logging.getLogger(__name__)
TEST_DIRECTORY = os.path.join(os.path.dirname(__file__), "tests")

@pytest.mark.parametrize("project", ["AutomatedTesting"])
@pytest.mark.parametrize("launcher_platform", ['windows_editor'])
class TestAutomation(EditorTestSuite):

    enable_prefab_system = True

    @pytest.mark.test_case_id("C36529679")
    class AtomLevelLoadTest_Editor(EditorSharedTest):
        from Atom.tests import hydra_Atom_LevelLoadTest as test_module

    @pytest.mark.test_case_id("C36525657")
    class AtomEditorComponents_BloomAdded(EditorSharedTest):
        from Atom.tests import hydra_AtomEditorComponents_BloomAdded as test_module

    @pytest.mark.test_case_id("C32078118")
    class AtomEditorComponents_DecalAdded(EditorSharedTest):
        from Atom.tests import hydra_AtomEditorComponents_DecalAdded as test_module

The above example would run 3 tests in parallel, and as 1 batch (batch limits are 8 tests at a time, which can be changed inside the C:\git\o3de\Tools\LyTestTools\ly_test_tools\o3de\editor_test.py file but AR will assume this value of 8):

    # Function to calculate number of editors to run in parallel, this can be overriden by the user
    @staticmethod
    def get_number_parallel_editors():
        return 8

Since it batches the tests up using Editor as its base, all of the classes are "EditorX" classes where X is the additional naming for the class (i.e. class EditorTestSuite). These classes also define values that are closely tied to the Editor meaning that we should be able to implement this for MaterialEditor very easily by simply using the same objects and then making sure it targets MaterialEditor instead of Editor when launching the tests:

1. Find the entry point where "Editor" is launched in the code then provide the user an option to utilize "MaterialEditor" instead. This appears to be primarily in the class EditorTestClass.collect().make_test_func() method or _run_parallel_batched_tests function, specifically the editor parameter would need to reference MaterialEditor instead. This editor parameter feeds into all of the other function calls within the script, so will work seamlessly with MaterialEditor once the switch is made.
2. Update all Editor hard coded references such as editor.log to change based on the executable used by the test.
3. Add some feature optimizations to match the options available to us, such as changing use_null_renderer to something like use_rhi and then pass in the options null, dx12, or vulkan for instance. Basically updating the code to be more agnostic so that it supports both Editor and MaterialEditor options rather than fit a narrowly defined run for non-GPU null renderer tests.

Also, not every test has to be run in parallel and batched. In fact, GPU tests have to be run as a single test using the class EditorSingleTest object so this option will exist for any tests that can't be run in parallel for whatever reason. The added benefit of converting is primarily for the return codes which are the most solid test approach (previously we used log lines).

Initial support of this feature would be only for Windows, as that is our current standard. This could change in the future, but this RFC will not cover any of that design.

What are the advantages of the feature?

Faster test runs, more test stability (return codes instead of log lines), and easier than before to add new tests to an existing python test launcher script. All existing tests are also ready today to be converted in this way (and many have already).

What are the disadvantages of the feature?

The disadvantage will be that some unknowns remain as to how easy this "switch flip" would be from Editor to MaterialEditor - it looks easy to do when reviewing the code, but will it be easy when we change it and try to run MaterialEditor tests in this manner? That will be the large unknown this task work would unveil for us - though I predict it being workable.

How will this be implemented or integrated into the O3DE environment?

Very easily by simply adding to the existing code inside C:\git\o3de\Tools\LyTestTools\ly_test_tools\o3de\editor_test.py or the C:\git\o3de\Tools\LyTestTools\ly_test_tools\o3de\ directory. It uses files that already exist and functionality that has already been tested with Editor.exe extensively.

The hard part will be testing, hardening, and getting other users to adopt and update their code to match this new functionality. This would ideally be done using an e-mail linking to a wiki that details for users how to update their tests for this framework change so that nothing breaks unexpectedly (especially when it comes to renames, they're trivial but they can easily break a lot of existing code).

Are there any alternatives to this feature?

The alternative we had were log line tests which are not as reliable as return code tests.

How will users learn this feature?

There is already an existing internal wiki, and we would probably have need of a public one since this will be publicly available. However, since the demand for test tools has appeared low and most of our sig-testing meetings are usually only internal participants, I don't think a public wiki would be required initial - though it certainly wouldn't hurt in the long run.

Are there any open questions?

An open question is whether GPU tests will ever be able to run as parallel & batched tests. So far it looks like the answer is no, but these types of tests are often cumbersome in automated testing so are usually avoided or greatly reduced in volume compared to other types of tests since they are considered "front end tests". "Front end tests" are usually the slowest and most prone to race conditions, so even though this is a problem I believe it will always be one regardless of which automated test tool we choose.

[smurly]

I think some features to work on would include expanding the Boolean use_null_renderer to be a specify RHI default being -rhi=null but also supporting options -rhi=dx12 and -rhi=vulkan additional settings would be needed on other systems than Windows. making RHI a parameterized pytest option so that tests can hit multiple RHI options (dx12, vulkan) passed in as decorator @pytest.mark.parametrize('rhi', ['dx12', 'vulkan']) GPU tests use EditorSingleTest mostly because they have external setup/teardown needs which can't be handled by the batched/parallel classes. If GPU tests can be refactored to entirely contain the function of the test within the Editor Python Binding (hydra) test script then in theory it would be possible to use EditorBatchedTest. Parallel would likely not be an option since non-active windows can have either no frame updates to rendering or reduced GPU access (options exist to allow out of focus render windows to have frame updates but not by default) Because MaterialEditor is similar to Editor we might be able to fit these into one classed home but single-responsibility principle suggests that a parent class with the common functionality should be factored out with two single-responsibility classes for Editor and ME inheriting and separate. core functions of the collection and execution do have editor specific design to them though. GameLauncher may have limited application through editor_test.py since the purpose is to pass test python scripts to an application under test (AUT) and recover the results back as both an exit code and parsed json result. In the case of GameLauncher no python interpreter is present. we could limit function to a generic execute AUT with command line arguments and handle exit code. that would limit the function to internally scripted test levels and not reuse Python Editor Bindings (hydra). Log monitoring (parsing json results) would be entirely different.

[jckand]

Is there an exhaustive list of the executables we DO want to support? I imagine they'll each need some custom handling, so we should at least outline them. All that's mentioned here is:

• Material Editor
• Game Launcher

[Kadino]

It's trivial to launch things in parallel or serial, and the update to these scripts should be straightforward... The question is more about having test scripts that live in the target executables, and managing CLI params to start up the tests that live in these targets.

Game Server Launcher is another important target, and both it and the Game Launcher should be testable via https://github.com/o3de/sig-testing/issues/20

[Kadino]

Good feedback from @smurly which relates to not having sufficient control of parameterization and pre/post conditions in EditorTest, to script actions in the external (non-editor) Python.

[AlexOteiza]

My two cents here:

Material editor is pretty similar to Editor.exe, it should easily support all the cmdline arguments as the editor(for instance —run pythontest), they are part of the same visual studio solution and share a lot of code.

For supporting material editor it should be as easy as modifying “editor.exe” string with “materialeditor.exe” and editor.log with material editor log in the EditorTestSuite. We could basically rename the class to O3deTestSuite and then subclass only overriding the app exe and log name.

Material editors and editors.exe could also be ran in one along with other parallel(but ofc couldn’t be batched).

Apart from changing the application to run, the code could stay almost the same in order to support this feature.

Regarding launcher tests, that is a different beast. Unless the code has changed, the python editor interpreter is not implemented for the launcher. This is a feature that we lack in hydra and would need to be implemented properly

[jromnoa]

I'll be updating this RFC before the next sig-testing meeting this friday.

[Kadino]

It should be possible to avoid changing existing tests, and instead only change the class they derive from (and not its filename or name). This will simplify adoption to only require action when a user writes a new test targeting the MaterialEditor. They could construct a MaterialEditorTest wrapper to point at their test, in a similar way as the current EditorTest wrapper is used. (MaterialEditorSharedTest MaterialEditorSingleTest, etc.)

Along that line, I think it is necessary to have a batching system which marks a test as specific to the MaterialEditor, as MaterialEditor and Editor tests cannot be batched together inside the same executable (MaterialEditor or Editor). Executing MaterialEditor and Editor tests in parallel may make sense, but there's a separate issue of cross-module parallel/batching not currently being supported (I think this RFC should leave that feature out of scope). It's probably best to avoid duplicating the batch-and-parallel pytest plugin, and to make it more generic to handle additional test-types.

What is the format of tests that will execute inside the MaterialEditor? Does the MaterialEditor already have a dependency on Python and the EditorPythonBindings?

[AlexOteiza]

There is no need to change the tests. My recommended course of action is: 1) Rename EditorTestSuite to abstract O3deTestSuite, which needs two things to be implemented by a child class: an executable name and a log name. 2) Create EditorTestSuite as a derivation of O3deTestSuite that sets application as editor.exe and log name editor.log 3) Create MaterialEditorTestSuite that sets application as materialeditor.exe and material log.

Same goes with EditorSingleTest and so on. I’d create O3deSingleTest and then EditorSingleTest+MaterialEditorSingleTest, this way editor and material can have specific options that only apply to them

All the existing tests will still reference EditorTestSuite so no code change will be needed.

@Kadino Yes, material tests use hydra as well. They use the exact same approach as editor tests. The only difference is that material editor is a separate app from editor

Regarding of batching material and editor tests together. That doesn’t make any sense because the essence of batching is to batch all the tests in the same application. Since editor and material are different applications it is impossible to do that

[smurly]

There is an early draft RFC being crafted for a new tool Material Canvas that may need to fit into this effort later. I don't have any details, but it would likely be a distinct executable with python bindings. This would mesh well with the concept of implementing from O3deTestSuite base class

[Kadino]

From the recent SIG-Testing meeting, this RFC should be edited to be more specific about what it proposes. However the thrust of it is well-formed, and this should move forward.

Existing classes should not be renamed and should extend a new generic class, and only the framework itself should be made more generic.

@Kadino to reach out to SIG-Core to discuss how the the EditorPythonBindings will continue to be extended, and its generic use. @jromnoa please update the RFC description to be more clear on what it proposes, removing language such as It should ideally [...] (reduce ambiguity, but this does not need to include descriptions of every engineering task)

[jromnoa]

Updated to remove ambiguous language as well as remove references to renaming objects. Work will begin on this in our current sprint for Atom.

[jromnoa]

This PR is in draft now, it has one remaining issue before it is eligible for a proper review and merge: https://github.com/o3de/o3de/pull/9286

The issue is the LyTestTools MaterialEditor launcher call for get_returncode() and it not returning a 0 or non-zero value like the Editor launcher does. This is currently being investigated for a fix.

[Kadino]

Persisted this accepted RFC to https://github.com/o3de/sig-testing/blob/main/rfcs/material_editor_test_tools.md

[jromnoa]

This is done now and sample MaterialEditor tests exist within Atom. See here: https://github.com/o3de/o3de/blob/development/AutomatedTesting/Gem/PythonTests/Atom/TestSuite_Main_Null_Render_MaterialEditor_01.py

Documentation exists detailing the implementation here (and its going to be updated to the latest code changes this week or early next week): https://github.com/o3de/o3de/wiki/Multitest-Framework-(Running-multiple-tests-in-one-executable)