Proposed RFC Feature editor_test.py supports pytest parameterization passed through to collected execution.

smurly commented 2 years ago

Summary:

The current framework for test execution utilizes collection concepts implemented in .\o3de\Tools\LyTestTools\ly_test_tools\o3de\editor_test.py. Tests and suites implement classes from editor_test.py which can then be dynamically collected at runtime for execution. Because these suites and test classes subclass parent classes which are used in the collection representation, dynamic pytest paramerterization is not currently supported. It is desirable that we add customizable parameterization to editor_test.py TestSuite and SingleTest classes so that more complex test concepts can be created. Render Hardware Layer (RHI) should replace or augment use_null_renderer as an option since -rhi=null is simply a specific rhi requested.

What is the relevance of this feature?

Tests may need to specify a reusable parameter (i.e. level name) or execute their common code with different command line parameters (i.e. specifying an -rhi=). Currently a duplicate test script must be created for each variation.

Feature design description:

editor_test.py test classes and suite classes would handle standard pytest parameterization decoration. mechanisms would be available to define custom fixtures as allowable by either StandaloneTest or batch/parallel test classes. The default use_null_renderer behavior should be a parameterized rhi call that can be replaced with specified list of desired rhi such as ["dx12", "vulkan"]. Backwards compatability param use_null_renderer could remain. Any number of other command line argument parameters should be possible to pass through to the application under test.

Technical design description:

user defined classes which implement any of EditorTestSuite, EditorSingleTest, EdiditorSharedTests, or other parent classes used by collection will allow standard pytest decoration.
- There will need to be a way to only batch tests requesting the same editor arguments together. Possibly only supporting parameterization with SingleTest (and perhaps ParallelTest).
collection routines in editor_test will pass through parameterized decoration so that test execution will include the options
multiple execution of tests through parameterization will occur and be reflected in test suite summary results.

What are the advantages of the feature?

pytest typically is used with parameterization to avoid duplicate test code or provide for execution time options for tests.

What are the disadvantages of the feature?

Collection causes a difficult situation for passing parameterization. Some test scenarios like batching may not permit parameterization since many tests share one editor session. Batched tests may need to be grouped by parameters or excluded from parameterization.

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

this should be implemented in a way that standard pytest parameterization techniques would be familiar to a user of the test framework to some extent.

Are there any alternatives to this feature?

No. only a static assortment of parameters exist for some of the test classes such as workspace (a class containing methods and information about the execution environment)
Providing a mechanism to parameterize within the Editor may mitigate the need for this, but does not solve the same test writing issues.

How will users learn this feature?

Example tests will be written showing how this can be used. Most likely I envision some Atom tests using rhi parameterization to replace existing hard-coded rhi option calls. Not having this feature increases the difficulty to write tests.

Are there any open questions?

Kadino commented 2 years ago

This is an example of how this parameterization fails: https://github.com/o3de/o3de/issues/8818

Kadino commented 2 years ago

This behavior is essentially a bug in the pytest collection plugin used by EditorTest, and would need a redesign to work with parameterization.

jromnoa commented 2 years ago

After discussing this more with @AMZN-scspaldi I think this might not be worth doing. There's no additional benefit to this change other than re-enabling the automatic pytest decorator collection for tests and lowers the amount of code needed to be written to add new tests. While these changes would be good, I feel they conflict too much with the manual collections that occur in https://github.com/o3de/o3de/blob/development/Tools/LyTestTools/ly_test_tools/o3de/multi_test_framework.py

The effort this has been taking to get both manual and automatic pytest parameter collection to work doesn't seem worth this small bit of value we will gain. It would be more relevant if we utilized @pytest.mark.parametrize for our test values and test code, but only our test launcher scripts deal with pytest code. The actual testing is done by the python behaviorContext bindings scripts which emulate Editor/MaterialEditor/MaterialCanvas/etc. C++ functionality and are launched inside these applications. So for us, the benefit from this change is minimal vs. the effort required to make it happen.

I don't know if we should scrap this, but it could be a longterm "nice to have" maybe if we start wanting to utilize @pytest.mark.parametrize magical pytest decorator collection in the future. For now, we should stick to what works with manual collection unless someone else can present a more solid case for implementing this sooner.

jromnoa commented 2 years ago

I just reviewed https://github.com/o3de/o3de/issues/8818 and see the case presented there. Unfortunately there is no clear solution to this that will work for all test cases. It would be a nice QoL improvement for engineers to be able to use this with CTest commands as the linked GHI mentions but we can make alternatives such as using a :loop in a batch file and running the batch file then reviewing the output afterwards. It's not ideal, but I also don't think we NEED to have this change ASAP, it's more of a "nice to have" QoL change.

I guess the priority isn't critical either though, so not sure on when this would go in.

AMZN-scspaldi commented 2 years ago

I took care of https://github.com/o3de/o3de/issues/8818 as it was a misunderstanding by the user. I closed the issue to prevent any further confusion.

So the value of this is even lower than we thought, as issue 8818 does not apply.

smurly commented 1 year ago

this rfc is not about execution_number (ignore 8818). it's about a general inability to pass parameterized pytest through multitest collection. we would like to be able to parameterize editor launch params such as rhi (dx12, vulkan, etc.) currently we must duplicate the same test multiple times with different names and the single differing launch parameter. the only pytest marks that seem to work at this point are general xfail, skip and the sort. we would like to parameterize editor launch parameters at either the test suite level or as only SingleTest objects. this would allow for an editor session to run rhi=dx12 and then the same test(s) run again as rhi=vulkan with a new session without duplicating tests in the pytest script.

Kadino commented 1 year ago

Currently do not plan to pursue a fix to this

o3de / sig-testing