DAOS-XXXXX test: refactor NLT to make it more readable and its results easier to understand by mchaarawi · Pull Request #18808 · daos-stack/daos

mchaarawi · 2026-08-07T03:53:02Z

NLT was a single ~7,000-line script whose results were scattered across JUnit xml, three warnings json files, valgrind xml and a raw log tarball, so finding the one issue behind a CI failure meant cross-referencing several artifacts. This reworks NLT for both readability and result triage without changing what it tests or how CI invokes it.

Reporting: NLT now writes a single human-readable summary (nlt-summary.md, also printed to the console and archived by CI). It lists the verdict and counts, each failed test with the DFuse/DAOS log lines that caused it quoted inline plus a logfile:line reference, remaining log-analysis findings grouped per test (with a server-wide group for shared-server logs), valgrind notes and the slowest tests. Findings are correlated to the owning test via a per-thread active-test context, since POSIX tests run in parallel. The existing junit/json artifacts are unchanged for the Jenkins plugins. Controlled by --summary (default on, "" disables).

Suites: add --suite {ci,manual,all}. Tests too slow or disk-hungry for CI (stable_cont_inode, test_dfs_check, test_alloc_pil4dfs_ls) are no longer dead/commented code in the CI path; they run only under --suite manual|all. xtest_stable_cont_inode is renamed manual_stable_cont_inode and discovered via PosixTests.generate_manual_test_list(); --test list shows a manual section.

Structure: split node_local_test.py into a dependency-ordered nlt/ package (base, config, reporting, logging_utils, client, server, dfuse, helpers, posix_tests, fault_injection, special_tests, runner, cli). node_local_test.py is now a thin shim so ci/unit scripts and developer invocation are unchanged. Add utils/nlt/README.md, include the package in .dockerignore, and archive nlt-summary.md from the NLT and Fault-injection stages.

skip-test-hardware: true

Steps for the author:

Commit message follows the guidelines.
Appropriate Features or Test-tag pragmas were used.
Appropriate Functional Test Stages were run.
At least two positive code reviews including at least one code owner from each category referenced in the PR.
Testing is complete. If necessary, forced-landing label added and a reason added in a comment.

After all prior steps are complete:

Gatekeeper requested (daos-gatekeeper added as a reviewer).

github-actions · 2026-08-07T03:53:19Z

Errors are Title of PR is too long,Ticket number suffix is not a number. See https://daosio.atlassian.net/wiki/spaces/DC/pages/11133911069/Commit+Comments,Unable to load ticket data
https://daosio.atlassian.net/browse/DAOS-XXXXX

NLT was a single ~7,000-line script whose results were scattered across JUnit xml, three warnings json files, valgrind xml and a raw log tarball, so finding the one issue behind a CI failure meant cross-referencing several artifacts. This reworks NLT for both readability and result triage without changing what it tests or how CI invokes it. Reporting: NLT now writes a single human-readable summary (nlt-summary.md, also printed to the console and archived by CI). It lists the verdict and counts, each failed test with the DFuse/DAOS log lines that caused it quoted inline plus a logfile:line reference, remaining log-analysis findings grouped per test (with a server-wide group for shared-server logs), valgrind notes and the slowest tests. Findings are correlated to the owning test via a per-thread active-test context, since POSIX tests run in parallel. The existing junit/json artifacts are unchanged for the Jenkins plugins. Controlled by --summary (default on, "" disables). Suites: add --suite {ci,manual,all}. Tests too slow or disk-hungry for CI (stable_cont_inode, test_dfs_check, test_alloc_pil4dfs_ls) are no longer dead/commented code in the CI path; they run only under --suite manual|all. xtest_stable_cont_inode is renamed manual_stable_cont_inode and discovered via PosixTests.generate_manual_test_list(); --test list shows a manual section. Structure: split node_local_test.py into a dependency-ordered nlt/ package (base, config, reporting, logging_utils, client, server, dfuse, helpers, posix_tests, fault_injection, special_tests, runner, cli). node_local_test.py is now a thin shim so ci/unit scripts and developer invocation are unchanged. Add utils/nlt/README.md, include the package in .dockerignore, and archive nlt-summary.md from the NLT and Fault-injection stages. Skip-unit-test: true Skip-func-test-vm: true skip-test-hardware: true skip-unit-test-memcheck: true skip-unit-test-bdev: true Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

daltonbohning

How do you feel about moving utils/nlt to src/tests/nlt?

mchaarawi · 2026-08-07T15:49:48Z

How do you feel about moving utils/nlt to src/tests/nlt?

yea i guess that makes sense

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

mjmac · 2026-08-07T17:11:51Z

@mchaarawi: Interesting... Long overdue! Do you want me to look at reworking #18779 as a PR for this? I think with your fix for the deadlock issue it will be less important to get the watchdog landed, but I think it would be super useful to have going forward.

mchaarawi · 2026-08-07T17:27:51Z

@mchaarawi: Interesting... Long overdue! Do you want me to look at reworking #18779 as a PR for this? I think with your fix for the deadlock issue it will be less important to get the watchdog landed, but I think it would be super useful to have going forward.

yes that makes sense and would be appreciated if i don't have to rebase this PR.
please wait a little to get reviews/approval from all folks here on my PR so you wouldn't have to rebase a lot on requested changes.

Skip-unit-test: true skip-unit-test-memcheck: true skip-unit-test-bdev: true skip-functional: true Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

daltonbohning

Still working through this. Nothing blocking so far. Just leaving comments for future improvements.

daltonbohning · 2026-08-07T17:54:38Z

+# The NLT implementation lives in src/tests/nlt; ship it preserving the relative path so the
+# utils/node_local_test.py shim can import it.
+rsync -R -rlpt -z -e "ssh $SSH_KEY_ARGS" src/tests/nlt jenkins@"$NODE":build/


Future: we could define NLT more as a package so this is not necessary

daltonbohning · 2026-08-07T18:01:08Z

+            elif size.endswith('GiB'):
+                size = int(size[:-3])
+                size *= (1024 * 1024 * 1024)
+            self.max_log_size = int(size)


Future: catch ValueError and raise a more clear exception

daltonbohning · 2026-08-07T18:03:07Z

+            break
+        file_self = os.path.dirname(file_self)
+        if file_self == '/':
+            raise NLTestFail('build file not found')


Nit

Suggested change

raise NLTestFail('build file not found')

raise NLTestFail('.build_vars.json file not found')

daltonbohning · 2026-08-07T18:06:23Z

+    rc = run_daos_cmd(conf, cmd)
+
+    assert rc.returncode == 0
+
+    print(rc)


Nit: maybe should print before asserting in case there is useful info for debug

daltonbohning · 2026-08-07T18:10:59Z

+        try:
+            self._outputs[thread_id] += value
+        except KeyError:
+            self._outputs[thread_id] = value


Future: maybe make sure value is the expected type because technically this could happen and would error

write(4) # valid write('string') # cannot int + str

daltonbohning · 2026-08-07T18:17:30Z

+    """Setup and import the log tracing code"""
+    # Try and pick this up from the src tree if possible; src/tests/ is the parent of this package.
+    file_self = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+    logparse_dir = join(file_self, 'ftest/cart/util')


Future: NLT using a file under ftest implies that file should not live under ftest

daltonbohning · 2026-08-07T18:20:41Z

+        return "%.1f%s%s" % (num, 'Yi', suffix)
+
+    if os.path.exists(f'{filename}.old'):
+        raise NLTestFail('Log file exceeded max size')


Nit. Include name

Suggested change

raise NLTestFail('Log file exceeded max size')

raise NLTestFail(f'Log file exceeded max size: {filename}')

daltonbohning · 2026-08-07T18:26:02Z

+from .base import get_active_test
+
+
+class WarningsFactory():


Future: generally more exception handling in helper functions.

daltonbohning · 2026-08-07T18:32:23Z

+        text = '\n'.join(lines) + '\n'
+        with open(filename, 'w') as sfd:
+            sfd.write(text)


Future: check if lines is large enough to warrant using sfd.writelines instead

daltonbohning · 2026-08-07T18:38:35Z

+    if args.suite == 'manual':
+        raw_test_list = PosixTests.generate_manual_test_list()
+    elif args.suite == 'all':
+        raw_test_list = PosixTests.generate_test_list() + PosixTests.generate_manual_test_list()
+    else:
+        raw_test_list = PosixTests.generate_test_list()


Nit: explicitly handle ci case and error otherwise

Skip-unit-test: true skip-unit-test-memcheck: true skip-unit-test-bdev: true skip-functional: true Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

mchaarawi force-pushed the mschaara/nlt_refactor branch from bf5cf16 to 9018ef1 Compare August 7, 2026 04:01

mchaarawi changed the title ~~DAOS-XXXXX test: make NLT readable and its results easy to understand~~ DAOS-XXXXX test: refactor NLT to make it more readable and its results easier to understand Aug 7, 2026

mchaarawi force-pushed the mschaara/nlt_refactor branch from 9018ef1 to f97406f Compare August 7, 2026 13:52

mchaarawi force-pushed the mschaara/nlt_refactor branch from f97406f to 1c99c98 Compare August 7, 2026 14:13

daltonbohning reviewed Aug 7, 2026

View reviewed changes

mchaarawi mentioned this pull request Aug 7, 2026

DAOS-19427 dfuse: defer cache invalidations off the worker pool #18802

Open

6 tasks

daltonbohning reviewed Aug 7, 2026

View reviewed changes

Comment thread utils/nlt/base.py Outdated

Comment thread utils/nlt/base.py Outdated

Comment thread utils/nlt/cli.py Outdated

Comment thread utils/nlt/cli.py Outdated

Comment thread utils/nlt/cli.py Outdated

Comment thread utils/nlt/helpers.py Outdated

address feedback

16a20a7

Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

mchaarawi requested review from daltonbohning, phender and ryon-jensen August 7, 2026 17:28

move nlt to src/test

916e013

Skip-unit-test: true skip-unit-test-memcheck: true skip-unit-test-bdev: true skip-functional: true Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

mchaarawi force-pushed the mschaara/nlt_refactor branch from dc991de to 916e013 Compare August 7, 2026 17:31

daltonbohning reviewed Aug 7, 2026

View reviewed changes

address qick NITs

cfca9c8

Skip-unit-test: true skip-unit-test-memcheck: true skip-unit-test-bdev: true skip-functional: true Signed-off-by: Mohamad Chaarawi <mohamad.chaarawi@hpe.com>

	raise NLTestFail('build file not found')
	raise NLTestFail('.build_vars.json file not found')

	raise NLTestFail('Log file exceeded max size')
	raise NLTestFail(f'Log file exceeded max size: {filename}')

Uh oh!

Conversation

mchaarawi commented Aug 7, 2026

Steps for the author:

After all prior steps are complete:

Uh oh!

github-actions Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daltonbohning left a comment

Choose a reason for hiding this comment

Uh oh!

mchaarawi commented Aug 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mjmac commented Aug 7, 2026

Uh oh!

mchaarawi commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

daltonbohning left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Aug 7, 2026 •

edited

Loading

mchaarawi commented Aug 7, 2026 •

edited

Loading