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

Fix/multitest parting revision #1097

Merged
merged 1 commit into from
Jun 20, 2024
Merged

Conversation

zhenyu-ms
Copy link
Contributor

Bug / Requirement Description

Clearly and concisely describe the problem.

Solution description

use testcase-level round-robin scheduling for multitest parting (maybe more to go)

Checklist:

  • Test
  • Example (both test_plan.py and .rst)
  • Documentation (API)
  • News fragment present for release notes
  • MS info leakage check
  • For new driver: driver index page
  • For new assertion: ui/pdf/std renderers, documentation
  • For new cmdline arg: documentation

@zhenyu-ms zhenyu-ms requested a review from a team as a code owner June 6, 2024 01:37
Copy link
Contributor

@Pyifan Pyifan left a comment

Choose a reason for hiding this comment

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

I think if we change the splitting logic, we need to change merge logic too?

"reason": data["reason"],
"strict": data["strict"],
}
for testcase in testcases_to_run:
Copy link
Contributor

Choose a reason for hiding this comment

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

why are we making this change, i think the original code is fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah i just want to remove extra if statements


def disassemble(self):
c = []
for u in copy.copy(list(self._index.keys())):
Copy link
Contributor

Choose a reason for hiding this comment

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

just a shallow copy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will fix 😵

@@ -392,6 +392,11 @@ def get_test_context(self):
ctx = []
sorted_suites = self.cfg.test_sorter.sorted_testsuites(self.cfg.suites)

if hasattr(self.cfg, "xfail_tests") and self.cfg.xfail_tests:
Copy link
Contributor

Choose a reason for hiding this comment

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

set self.cfg.xfail_tests default value to {} ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(i'd like to pretend that) self.cfg is immutable 😄

report.definition_name,
category=report.category,
)
# here `report` must be an empty MultiTest report since
Copy link
Contributor

Choose a reason for hiding this comment

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

This part of comment needs a bit of refinement, to be more concise.

@zhenyu-ms zhenyu-ms merged commit 5075146 into morganstanley:main Jun 20, 2024
15 checks passed
@zhenyu-ms zhenyu-ms deleted the re-mt-parts branch June 21, 2024 03:43
zhenyu-ms added a commit to zhenyu-ms/testplan that referenced this pull request Jun 21, 2024
zhenyu-ms added a commit to zhenyu-ms/testplan that referenced this pull request Jul 4, 2024
zhenyu-ms added a commit to zhenyu-ms/testplan that referenced this pull request Jul 29, 2024
zhenyu-ms added a commit that referenced this pull request Jul 30, 2024
* impl round-robin-like parts merging

* address review comments in #1097

* dedup

* some refactor

* add newsfrag; adjust comments
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