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

Hierarchical based exhaustive sort #119

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

janag123
Copy link

@janag123 janag123 commented Jan 10, 2024

Description

This pull request enhances the pytest plugin by introducing the --exhaustive-sort option, designed to optimize test case execution order for improved efficiency. When collecting test cases, pytest traditionally organizes them class-wise per parameter set (eg. of parameter set: [l3vpn-6pe-6vpe-common_setup_transport0]) , leading to potential redundancy in setup and teardown operations.
The newly added custom_exhaustive_sort method, utilized in the pytest_collection_modifyitems function, addresses this inefficiency. It reorders test cases hierarchically, ensuring that all test cases for a given parameter set are exhausted before moving on to the next set. This approach optimizes execution time by minimizing redundant setup and teardown operations.

Example 1:
Command used:

pytest simplified_example.py --exhaustive-sort

Simple example File: simple_example(.py).txt
Log: simple_example.log

File structure:

class Test_SuperClass:
    @pytest.fixture(scope="class", params=["transport0", "transport1"], autouse=True)

    class Test_A:
        def test1()

    class Test_B:
        def test2()

Before Sorting: (pytest generated order)

ClassA_Test1[transport0]
ClassA_Test1[transport1]
ClassB_Test2[transport0]
ClassB_Test2[transport1]

After Sorting:

ClassA_Test1[transport0]
ClassB_Test2[transport0]
ClassA_Test1[transport1]
ClassB_Test2[transport1]

This optimization is particularly beneficial when dealing with large test suites, as it reduces the overall execution time.

Example 2:
An example output is also attached for reference where this sorting method was run on a cafyap file.
Command used:


pytest -s l3vpn_intra_as_ap.py --script-args "{'capability_template': 'l3vpn_template.json', 'test_input_py_file':'l3vpn_intra_as_test_input.py'}" -T /ws/oliviaj-ott/cafyap/l3vpn/l3_fib/F6-SF-DEV-POD16.json --disable-warnings --collect-only --exhaustive-sort

Log:
l3vpn_intra_as_ap.log

@shreyashah
Copy link
Collaborator

Please add logs and from future maintainability context, u may want to define what exhaustive sort means by providing a sample

@janag123 janag123 changed the title Hierarchal based exhaustive sort Hierarchical based exhaustive sort Jan 10, 2024
@nishantupadhyay123
Copy link
Collaborator

I am not sure what is the feature here .

@janag123
Copy link
Author

I have added more description and an example output log.

@nishantupadhyay123
Copy link
Collaborator

thanks for the context. we need more testing :

  1. with selective testcases enabled.
  2. without selective testcases
  3. with existing AP's the way it is written behaves with this option enabled.
  4. I think you added the UT file already. Can you add the file here in the repo.

Comment on lines +665 to +671
while copy_of_items:
param = extract_params(copy_of_items[0])
if param:
param_pattern = f"[{param}"
matching_items = [item for item in copy_of_items if param_pattern in item.nodeid]
sorted_items.extend(matching_items)
copy_of_items = [item for item in copy_of_items if item not in matching_items]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I am lost here. This is copy of an array and sort is performed over items. I get that the genral sorting algorithms wont apply. But what is 671.

Copy link
Author

Choose a reason for hiding this comment

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

The while loop is supposed to go on until the copy is completely empty. Matching items are the items being added into the sorted array (line 670). After that, I am removing all the matching items from the copy which I have added into the sorted array.

Copy link
Author

Choose a reason for hiding this comment

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

I.e, in line 671 I am removing those items.

if config.option.exhaustive_sort:
log.info("Exhaustive sorting of TCs enabled")
sorted_items = custom_exhaustive_sort(items)
if len(sorted_items) == len(items):
Copy link
Collaborator

Choose a reason for hiding this comment

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

in which cases this will never be true ? this should sort anyway.
is it for the non-fixture things

Copy link
Author

Choose a reason for hiding this comment

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

I haven't found a case where it will not be true but I added it just in case as a roll back to default order if the sorting misbehaves or fails resulting in lesser number or duplication of test cases in the sorted array.

@nishantupadhyay123
Copy link
Collaborator

seems fine , Jatin. We need test logs for several options I suggested. Ping me for any help. Post that it is good to go.

@nishantupadhyay123
Copy link
Collaborator

No tests done and closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants