Skip to content

[bugfix] Fix skipping of test cases in case of unresolved dependencies #1751

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Feb 8, 2021
Merged

[bugfix] Fix skipping of test cases in case of unresolved dependencies #1751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f8cd094
Fix skipping of test cases in case of unresolved dependencies
Feb 6, 2021
9394851
Fine tune implementation and fix output
Feb 6, 2021
571d6de
Update documentation
Feb 6, 2021
ec6995f
Fix user name in tutorial of dependencies
Feb 8, 2021
ce007fd
Merge branch 'master' into misc/unit-test-dangling-deps
Feb 8, 2021
e53f38c
Merge branch 'master' into misc/unit-test-dangling-deps
Feb 8, 2021
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
84 changes: 43 additions & 41 deletions docs/tutorial_deps.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,12 @@ Here is the output when running the OSU tests with the asynchronous execution po
[ReFrame Setup]
version: 3.4-dev2 (rev: 56c6c237)
command: './bin/reframe --system=daint -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -r'
launched by: karakasv@dom101
working directory: '/users/karakasv/Devel/reframe'
launched by: user@daint101
working directory: '/users/user/Devel/reframe'
settings file: 'tutorials/config/settings.py'
check search path: '/users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py'
stage directory: '/users/karakasv/Devel/reframe/stage'
output directory: '/users/karakasv/Devel/reframe/output'
check search path: '/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py'
stage directory: '/users/user/Devel/reframe/stage'
output directory: '/users/user/Devel/reframe/output'

[==========] Running 8 check(s)
[==========] Started on Mon Jan 25 19:34:09 2021
Expand Down Expand Up @@ -234,7 +234,7 @@ For example, if we select only the :class:`OSULatencyTest` for running, ReFrame
[ReFrame Setup]
version: 3.3-dev2 (rev: 8ded20cd)
command: './bin/reframe -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l'
launched by: user@dom101
launched by: user@daint101
working directory: '/users/user/Devel/reframe'
settings file: 'tutorials/config/settings.py'
check search path: '/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py'
Expand All @@ -256,48 +256,50 @@ As a result, its immediate dependency :class:`OSUBuildTest` will be skipped, whi

.. code-block:: console

./bin/reframe --system=daint:gpu -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l
./bin/reframe -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l

.. code-block:: none

[ReFrame Setup]
version: 3.4-dev2 (rev: c1a364f3)
command: './bin/reframe --system=daint:gpu -C tutorials/config/settings.py -c tutorials/deps/osu_benchmarks.py -n OSULatencyTest -l'
launched by: karakasv@dom101
working directory: '/users/karakasv/Devel/reframe'
version: 3.5-dev0 (rev: 93948510)
command: './bin/reframe -C tutorials/config/settings.py --system=daint:gpu -c tutorials/deps/osu_benchmarks.py -l'
launched by: user@daint101
working directory: '/users/user/Devel/reframe'
settings file: 'tutorials/config/settings.py'
check search path: '/users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py'
stage directory: '/users/karakasv/Devel/reframe/stage'
output directory: '/users/karakasv/Devel/reframe/output'

./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'gnu') -> 'OSUDownloadTest'; skipping dependent test cases:
./bin/reframe: - ('OSUBuildTest', 'daint:gpu', 'gnu')
./bin/reframe: - ('OSUBandwidthTest', 'daint:gpu', 'gnu')
./bin/reframe: - ('OSUAllreduceTest_16', 'daint:gpu', 'gnu')
./bin/reframe: - ('OSULatencyTest', 'daint:gpu', 'gnu')
./bin/reframe: - ('OSUAllreduceTest_4', 'daint:gpu', 'gnu')
./bin/reframe: - ('OSUAllreduceTest_8', 'daint:gpu', 'gnu')
./bin/reframe: - ('OSUAllreduceTest_2', 'daint:gpu', 'gnu')
./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'intel') -> 'OSUDownloadTest'; skipping dependent test cases:
./bin/reframe: - ('OSUBuildTest', 'daint:gpu', 'intel')
./bin/reframe: - ('OSULatencyTest', 'daint:gpu', 'intel')
./bin/reframe: - ('OSUAllreduceTest_4', 'daint:gpu', 'intel')
./bin/reframe: - ('OSUAllreduceTest_16', 'daint:gpu', 'intel')
./bin/reframe: - ('OSUBandwidthTest', 'daint:gpu', 'intel')
./bin/reframe: - ('OSUAllreduceTest_2', 'daint:gpu', 'intel')
./bin/reframe: - ('OSUAllreduceTest_8', 'daint:gpu', 'intel')
./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'pgi') -> 'OSUDownloadTest'; skipping dependent test cases:
./bin/reframe: - ('OSUBuildTest', 'daint:gpu', 'pgi')
./bin/reframe: - ('OSUAllreduceTest_8', 'daint:gpu', 'pgi')
./bin/reframe: - ('OSUAllreduceTest_2', 'daint:gpu', 'pgi')
./bin/reframe: - ('OSUBandwidthTest', 'daint:gpu', 'pgi')
./bin/reframe: - ('OSUAllreduceTest_16', 'daint:gpu', 'pgi')
./bin/reframe: - ('OSUAllreduceTest_4', 'daint:gpu', 'pgi')
./bin/reframe: - ('OSULatencyTest', 'daint:gpu', 'pgi')
check search path: '/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py'
stage directory: '/users/user/Devel/reframe/stage'
output directory: '/users/user/Devel/reframe/output'

./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'gnu') -> 'OSUDownloadTest'
./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'intel') -> 'OSUDownloadTest'
./bin/reframe: could not resolve dependency: ('OSUBuildTest', 'daint:gpu', 'pgi') -> 'OSUDownloadTest'
./bin/reframe: skipping all dependent test cases
- ('OSUBuildTest', 'daint:gpu', 'intel')
- ('OSUBuildTest', 'daint:gpu', 'pgi')
- ('OSUAllreduceTest_4', 'daint:gpu', 'intel')
- ('OSUAllreduceTest_2', 'daint:gpu', 'intel')
- ('OSULatencyTest', 'daint:gpu', 'pgi')
- ('OSUBandwidthTest', 'daint:gpu', 'pgi')
- ('OSUAllreduceTest_8', 'daint:gpu', 'intel')
- ('OSUAllreduceTest_16', 'daint:gpu', 'pgi')
- ('OSUBuildTest', 'daint:gpu', 'gnu')
- ('OSUBandwidthTest', 'daint:gpu', 'intel')
- ('OSULatencyTest', 'daint:gpu', 'intel')
- ('OSUAllreduceTest_16', 'daint:gpu', 'intel')
- ('OSUAllreduceTest_8', 'daint:gpu', 'pgi')
- ('OSULatencyTest', 'daint:gpu', 'gnu')
- ('OSUBandwidthTest', 'daint:gpu', 'gnu')
- ('OSUAllreduceTest_2', 'daint:gpu', 'pgi')
- ('OSUAllreduceTest_4', 'daint:gpu', 'pgi')
- ('OSUAllreduceTest_8', 'daint:gpu', 'gnu')
- ('OSUAllreduceTest_4', 'daint:gpu', 'gnu')
- ('OSUAllreduceTest_2', 'daint:gpu', 'gnu')
- ('OSUAllreduceTest_16', 'daint:gpu', 'gnu')

[List of matched checks]

Found 0 check(s)
Log file(s) saved in: '/tmp/rfm-eiay984f.log'
Log file(s) saved in: '/tmp/rfm-hjit66h2.log'


Listing Dependencies
Expand Down Expand Up @@ -381,7 +383,7 @@ The following listing shows how the actual test cases dependencies are formed wh
<none>

Location:
/users/karakasv/Devel/reframe/tutorials/deps/osu_benchmarks.py
/users/user/Devel/reframe/tutorials/deps/osu_benchmarks.py

Maintainers:
<none>
Expand Down
44 changes: 26 additions & 18 deletions reframe/frontend/dependencies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def resolve_dep(src, dst):
# We use an ordered dict here, because we need to keep the order of
# partitions and environments
graph = collections.OrderedDict()
skipped_cases = []
unresolved_cases = []
for c in cases:
psrc = c.partition.name
esrc = c.environ.name
Expand All @@ -78,32 +78,40 @@ def resolve_dep(src, dst):
if when((psrc, esrc), (pdst, edst)):
c.deps.append(d)
except DependencyError as e:
getlogger().warning(f'{e}; skipping dependent test cases:')

# FIXME: we need to unit test this properly
skip_nodes = {c}
while skip_nodes:
v = skip_nodes.pop()
skipped_cases.append(v)
getlogger().warning(f' - {v}')
pruned_nodes = []
for u, adj in graph.items():
if v in adj:
skip_nodes.add(u)
pruned_nodes.append(u)

for u in pruned_nodes:
del graph[u]

getlogger().warning(e)
unresolved_cases.append(c)
continue

graph[c] = util.OrderedSet(c.deps)

# Skip also all cases that depend on the unresolved ones
skipped_cases = []
skip_nodes = set(unresolved_cases)
while skip_nodes:
v = skip_nodes.pop()
skipped_cases.append(v)
for u, adj in graph.items():
if v in adj:
skip_nodes.add(u)

# Prune graph
for c in skipped_cases:
# Cases originally discovered (unresolved_cases) are not in the graph,
# but we loop over them here; therefore we use pop()
graph.pop(c, None)

# Calculate in-degree of each node
for u, adjacent in graph.items():
for v in adjacent:
v.in_degree += 1

msg = 'skipping all dependent test cases\n'
for c in skipped_cases:
msg += f' - {c}\n'

if skipped_cases:
getlogger().warning(msg)

return graph, skipped_cases


Expand Down
23 changes: 17 additions & 6 deletions unittests/test_dependencies.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,25 +646,36 @@ def test_validate_deps_empty(exec_ctx):


def test_skip_unresolved_deps(make_test, temp_runtime):
#
# t0 t4
# ^ ^ ^
# / \ /
# t1 t2
# ^
# |
# t3
#

rt = temp_runtime(fixtures.TEST_CONFIG_FILE, system='sys0:p0')
next(rt)

t0 = make_test('t0')
t0.valid_systems = ['sys0:p1']
t1 = make_test('t1')
t2 = make_test('t2')
t3 = make_test('t3')
t3.valid_systems = ['sys0:p1']
t4 = make_test('t4')
t1.depends_on('t0')
t2.depends_on('t0')
t2.depends_on('t3')
t2.depends_on('t4')
t3.depends_on('t2')
deps, skipped_cases = dependencies.build_deps(
executors.generate_testcases([t0, t1, t2, t3])
executors.generate_testcases([t0, t1, t2, t3, t4])
)

assert len(skipped_cases) == 2
assert len(skipped_cases) == 6

skipped_tests = {c.check.name for c in skipped_cases}
assert skipped_tests == {'t2'}
assert skipped_tests == {'t1', 't2', 't3'}


def assert_topological_order(cases, graph):
Expand Down