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 parametrization reload #983

Merged
merged 5 commits into from
Aug 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed an issue whereas changes to a parametric testcases were not reflected on the interactive GUI upon reload.
41 changes: 23 additions & 18 deletions testplan/runnable/interactive/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -789,25 +789,30 @@ def reload(self, rebuild_dependencies=False):
def reload_report(self):
"""Update report with added/removed testcases"""
new_report = self._initial_report()
for mt in self.report.entries: # multitest level
for st_index in range(len(mt.entries)): # testsuite level
st = mt.entries[st_index]
new_st = new_report[mt.uid][st.uid]
for new_case_index in range(
len(new_report[mt.uid][st.uid].entries)
): # testcase level
for multitest in self.report.entries: # multitest level
for suite_index, suite in enumerate(multitest.entries):
new_suite = new_report[multitest.uid][suite.uid]
for case_index, case in enumerate(suite.entries):
try:
# Update new report testcase state
new_report[mt.uid][st.uid].entries[
new_case_index
] = st[
new_report[mt.uid][st.uid]
.entries[new_case_index]
.uid
]
except KeyError: # new testcase
pass
mt.entries[st_index] = new_st
if isinstance(case, TestGroupReport):
Copy link
Contributor

@Pyifan Pyifan Aug 15, 2023

Choose a reason for hiding this comment

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

Can we add some test coverage for this change? To prevent future regression. Otherwise this looks great!

for param_index, param_case in enumerate(
case.entries
):
try:
new_report[multitest.uid][suite.uid][
case.uid
].entries[param_index] = case[
param_case.uid
]
except KeyError:
continue
else:
new_report[multitest.uid][suite.uid].entries[
case_index
] = suite[case.uid]
except KeyError:
continue
multitest.entries[suite_index] = new_suite

def _setup_http_handler(self):
"""
Expand Down
95 changes: 89 additions & 6 deletions tests/unit/testplan/runnable/interactive/test_irunner.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Test the interactive test runner."""
from copy import deepcopy
from unittest import mock

import pytest
Expand All @@ -7,13 +8,14 @@
from testplan import report
from testplan import runnable
from testplan.common import entity
from testplan.report.testing.base import RuntimeStatus
from testplan.report import RuntimeStatus, TestCaseReport
from testplan.testing import filtering
from testplan.testing import multitest
from testplan.testing import ordering
from testplan.testing.multitest import driver
from testplan.runnable.interactive import base
from testplan.common.utils.path import default_runpath
from testplan.common.utils.testing import check_report
from testplan.runners.local import LocalRunner


Expand Down Expand Up @@ -142,8 +144,6 @@ def irunner():
@pytest.mark.parametrize("sync", [True, False])
def test_run_all_tests(irunner, sync):
"""Test running all tests."""
_check_initial_report(irunner.report)

ret = irunner.run_all_tests(await_results=sync)

# If the tests were run asynchronously, await the results.
Expand Down Expand Up @@ -337,12 +337,11 @@ def test_run_all_tagged_tests(tags, num_of_suite_entries):
irunner.teardown()


def _check_initial_report(initial_report):
def test_initial_report(irunner):
"""
Check that the initial report tree is generated correctly.

First, check that there are three top-level Test reports.
"""
initial_report = irunner.report
assert initial_report.status == report.Status.UNKNOWN
assert initial_report.runtime_status == report.RuntimeStatus.READY
assert len(initial_report.entries) == 3
Expand All @@ -366,3 +365,87 @@ def _check_initial_report(initial_report):
param_report = suite_report.entries[1]
assert isinstance(param_report, report.TestGroupReport)
assert len(param_report.entries) == 3


def test_reload_report(irunner):
"""
Tests report reload of the interactive handler.
"""
# We run one of the MultiTests, the suite for another, and a testcase
# and one of the parametrized testcases for the third.
irunner.run_test("test_1", await_results=True)
irunner.run_test_suite("test_2", "Suite", await_results=True)
irunner.run_test_case("test_3", "Suite", "case", await_results=True)
irunner.run_test_case_param(
"test_1",
"Suite",
"parametrized",
"parametrized__val_2",
await_results=True,
)
# Now we modify the reports forcefully
# In test_1, we take no action
# In test_2, we remove "parametrized" it needs to be added back, and we
# add "new_case" that should be removed
irunner.report["test_2"]["Suite"].entries.pop(1)
new_case = TestCaseReport(name="new_case", uid="new_case")
irunner.report["test_2"]["Suite"].entries.append(new_case)
# In test_3, we update "parametrized" by removing "3" and adding
# "4", the former will be added back the latter will be removed
irunner.report["test_3"]["Suite"]["parametrized"].entries.pop()
new_parametrized = TestCaseReport(
name="parametrized <val=4>",
uid="parametrized__val_4",
)
irunner.report["test_3"]["Suite"]["parametrized"].entries.append(
new_parametrized
)

# We preserve the current report
old_report = deepcopy(irunner.report)

# We reload and assert
irunner.reload_report()

for test in irunner.report:
Copy link
Contributor Author

@M6AI M6AI Aug 15, 2023

Choose a reason for hiding this comment

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

@Pyifan In a nutshell, this test does the following:

  • ensures that runtime status actually moves to ready upon change (this is functionally irrelevant tho)
  • checks whether the original report state got restored (after alterations with new cases and parametrizations or removals of either) and whether everything at testcase level that was run and kept have been preserved under the reload

Seems smoother with public setters than mocking some behavior using proper mocks.

# A MultiTest should reset to ready upon changes underneath
assert test.runtime_status == (
RuntimeStatus.FINISHED
if test.uid == "test_1"
else RuntimeStatus.READY
)
for suite in irunner.report[test.uid]:
# A testsuite should reset to ready upon changes underneath
assert suite.runtime_status == (
RuntimeStatus.FINISHED
if test.uid == "test_1"
else RuntimeStatus.READY
)
for index, entry in enumerate(suite):
# We need to check explicitly both "case" and "parametrized"
assert entry.uid == ("parametrized" if index else "case")
if entry.uid == "case":
check_report(
old_report[test.uid][suite.uid][entry.uid],
irunner.report[test.uid][suite.uid][entry.uid],
)
elif entry.uid == "parametrized":
for param_index, param in enumerate(entry):
assert (
param.uid
== [
f"parametrized__val_{i + 1}" for i in range(3)
][param_index]
)
if (test.uid == "test_1") or (
test.uid == "test_3"
and param.uid != "parametrized__val_3"
):
check_report(
old_report[test.uid][suite.uid][entry.uid][
param.uid
],
irunner.report[test.uid][suite.uid][entry.uid][
param.uid
],
)