tests: translated log-analyzer script from bash to python #15180

maykathm · 2025-03-07T09:46:53Z

This replaces the log-analyzer script from bash to python. The bash script, because of many calls to spread -list when getting the list of tasks to reexecute, could require upwards of 20 minutes to execute. The new Python script will call spread -list exactly once. This additionally adds some python unit tests to ensure the script's correctness.

codecov · 2025-03-07T10:05:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.17%. Comparing base (3141522) to head (7625dc9).
Report is 85 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #15180      +/-   ##
==========================================
+ Coverage   78.06%   78.17%   +0.11%     
==========================================
  Files        1183     1192       +9     
  Lines      158250   159589    +1339     
==========================================
+ Hits       123541   124763    +1222     
- Misses      27029    27099      +70     
- Partials     7680     7727      +47

Flag	Coverage Δ
unittests	`78.17% <ø> (+0.11%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-03-07T11:35:30Z

Fri Mar 21 11:07:03 UTC 2025

Spread tests skipped

bboozzoo

Has this already landed in snapd-testing-tools?

maykathm · 2025-03-10T09:52:03Z

Has this already landed in snapd-testing-tools?

Thanks for pointing that out. I opened a PR there canonical/snapd-testing-tools#57

zyga

Thank you for tackling this problem. This much welcome improvement.

I left a few quick comments but I'd really like to see type annotations and type-clean mypy --strict pass. Those little utilities tend to break from time to time and when one tries to reverse engineer what's going on it's often that the types fall apart in some edge cases - exactly what mypy can automatically help figure out.

I'm super happy to see unit tests, this is exactly what we should have always done.

I left some more comments on argument parsing details. I think you can really refactor that part and get very clean code with great user experience (--help and the like).

One word of caution, I'm not sure exactly if we are bound by python version requirements to run this (do we ever run this with ancient python?) I'd be really good to clearly document that in the log analyzer itself, as a longer code comment.

tests/lib/external/snapd-testing-tools/utils/log-analyzer

Signed-off-by: Zygmunt Krynicki <[email protected]>

This has remaining, unfixed type bug that needs to be discussed. Signed-off-by: Zygmunt Krynicki <[email protected]>

zyga · 2025-03-17T12:35:54Z

tests/lib/external/snapd-testing-tools/utils/log-analyzer

+from typing import Callable, TypedDict
+
+
+class SpreadLog(TypedDict):


I had a longer look on this and I think the fundamental problem is that the fields are all optional depending on the type. We really have an union of several types. This has become more evident as I started to look at tests.

zyga · 2025-03-17T12:37:27Z

@maykathm I've pushed a change that tries to illustrate the remaining type problem (and indeed a realization that the underlying data format is just weakly specified):

zyga@CFSR4-1:~/Projects/snapd/tests/lib/external/snapd-testing-tools/utils$ mypy --strict log-analyzer test_log_analyzer.py 
log-analyzer:53: error: TypedDict "SpreadLog_TypePhase" has no key "detail"  [typeddict-item]
test_log_analyzer.py:82: error: Missing key "stage" for TypedDict "SpreadLog_TypeResult"  [typeddict-item]
Found 2 errors in 2 files (checked 2 source files)

Have a look at the patches I've pushed to this branch please.

tests/lib/external/snapd-testing-tools/utils/log-analyzer

zyga · 2025-03-17T12:40:45Z

tests/lib/external/snapd-testing-tools/utils/log-analyzer

+def _get_detail_lines(
+    spread_logs: list[SpreadLog], log_condition_func: Callable[[SpreadLog], bool]
+) -> list[str]:
+    result = [log["detail"]["lines"] for log in spread_logs if log_condition_func(log)]


Do all the log "types" have a detail that has lines?

tests/lib/external/snapd-testing-tools/utils/log-analyzer

zyga · 2025-03-17T12:45:45Z

tests/lib/external/snapd-testing-tools/utils/log-analyzer

+    type: Literal["result"]
+    result_type: str
+    level: str
+    stage: str


I'd love to have a literal/enum that lists all known stages.

I believe the stage is read directly from the spread log so it is just the stages in spread: prepare, restore, execute

Thanks, I'll do a follow-up that tries to make this more explicit.

zyga · 2025-03-17T12:47:32Z

tests/lib/external/snapd-testing-tools/utils/log-analyzer

+    add_arguments(reexecute)
+    list_all.add_argument(
+        "exec_params",
+        help="This is the parameter used to run spread (something like this BACKEND:SYSTEM:SUITE)",


Nitpick, this is copy-pasted from and shares a sibling above. We should clarify that the list is comma-separated or use nargs='+' if this does not break external format constraints.

zyga

LGTM! Thank you!

I'd like to do a follow-up that moves some of the python types module as we will have more and more scripts that kind of do the same thing and I'd like to implement it fewer rather than more times.

sergiocazzolato · 2025-03-19T20:14:55Z

Thanks for this improvement. Also I think we need to clean the backends/systems we have in out spread.yaml because spread in order to list the jobs that match with the filter, calculates all the combinations and then apply the filter to all the jobs.

sergiocazzolato · 2025-03-19T20:15:30Z

Please, remember that we need to apply all the changes to snapd-testing-tools first, and then import on this branch those changes.

sergiocazzolato

thanks

sergiocazzolato · 2025-03-19T20:29:40Z

I blocked this pr until the change is imported from snapd-testing-tools

… quoted run tests

maykathm requested a review from sergiocazzolato March 7, 2025 09:47

bboozzoo reviewed Mar 10, 2025

View reviewed changes

tests: translated log-analyzer to python

7e92fa6

maykathm force-pushed the SNAPDENG-34422-fix-long-running-log-analyzer branch from 5d736e9 to 7e92fa6 Compare March 10, 2025 09:48

zyga reviewed Mar 10, 2025

View reviewed changes

tests: use sub parsers and type hints

807af21

maykathm force-pushed the SNAPDENG-34422-fix-long-running-log-analyzer branch from 4d6738e to 807af21 Compare March 11, 2025 13:47

maykathm requested a review from zyga March 13, 2025 08:15

zyga added 2 commits March 17, 2025 13:32

tests: re-format log-analyzer with black

adf0eaf

Signed-off-by: Zygmunt Krynicki <[email protected]>

tests: write more precise type for SpreadLog

64d668c

This has remaining, unfixed type bug that needs to be discussed. Signed-off-by: Zygmunt Krynicki <[email protected]>

zyga reviewed Mar 17, 2025

View reviewed changes

tests/lib/external/snapd-testing-tools/utils/log-analyzer Outdated Show resolved Hide resolved

zyga reviewed Mar 17, 2025

View reviewed changes

tests/lib/external/snapd-testing-tools/utils/log-analyzer Outdated Show resolved Hide resolved

zyga reviewed Mar 17, 2025

View reviewed changes

tests/lib/external/snapd-testing-tools/utils/log-analyzer Outdated Show resolved Hide resolved

zyga reviewed Mar 17, 2025

View reviewed changes

tests/lib/external/snapd-testing-tools/utils/log-analyzer Outdated Show resolved Hide resolved

zyga reviewed Mar 17, 2025

View reviewed changes

tests/lib/external/snapd-testing-tools/utils/log-analyzer Outdated Show resolved Hide resolved

zyga reviewed Mar 17, 2025

View reviewed changes

tests: minor changes and add unit tests for main

b74b6ff

maykathm requested a review from zyga March 19, 2025 13:44

zyga approved these changes Mar 19, 2025

View reviewed changes

sergiocazzolato approved these changes Mar 19, 2025

View reviewed changes

sergiocazzolato added the ⛔ Blocked label Mar 19, 2025

tests: use string input instead of nargs for exec_params to allow for…

7625dc9

… quoted run tests

maykathm added Skip spread Indicate that spread job should not run and removed ⛔ Blocked labels Mar 21, 2025

maykathm closed this Mar 21, 2025

maykathm reopened this Mar 21, 2025

maykathm merged commit 267e482 into canonical:master Mar 21, 2025
130 of 137 checks passed

maykathm deleted the SNAPDENG-34422-fix-long-running-log-analyzer branch March 21, 2025 13:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: translated log-analyzer script from bash to python #15180

tests: translated log-analyzer script from bash to python #15180

maykathm commented Mar 7, 2025

codecov bot commented Mar 7, 2025 •

edited

Loading

github-actions bot commented Mar 7, 2025 •

edited

Loading

bboozzoo left a comment

maykathm commented Mar 10, 2025

zyga left a comment

zyga Mar 17, 2025

zyga commented Mar 17, 2025

zyga Mar 17, 2025

zyga Mar 17, 2025

maykathm Mar 19, 2025

zyga Mar 19, 2025

zyga Mar 17, 2025

zyga left a comment

sergiocazzolato commented Mar 19, 2025

sergiocazzolato commented Mar 19, 2025

sergiocazzolato left a comment

sergiocazzolato commented Mar 19, 2025

		from typing import Callable, TypedDict


		class SpreadLog(TypedDict):

tests: translated log-analyzer script from bash to python #15180

tests: translated log-analyzer script from bash to python #15180

Conversation

maykathm commented Mar 7, 2025

codecov bot commented Mar 7, 2025 • edited Loading

Codecov Report

github-actions bot commented Mar 7, 2025 • edited Loading

Spread tests skipped

bboozzoo left a comment

Choose a reason for hiding this comment

maykathm commented Mar 10, 2025

zyga left a comment

Choose a reason for hiding this comment

zyga Mar 17, 2025

Choose a reason for hiding this comment

zyga commented Mar 17, 2025

zyga Mar 17, 2025

Choose a reason for hiding this comment

zyga Mar 17, 2025

Choose a reason for hiding this comment

maykathm Mar 19, 2025

Choose a reason for hiding this comment

zyga Mar 19, 2025

Choose a reason for hiding this comment

zyga Mar 17, 2025

Choose a reason for hiding this comment

zyga left a comment

Choose a reason for hiding this comment

sergiocazzolato commented Mar 19, 2025

sergiocazzolato commented Mar 19, 2025

sergiocazzolato left a comment

Choose a reason for hiding this comment

sergiocazzolato commented Mar 19, 2025

codecov bot commented Mar 7, 2025 •

edited

Loading

github-actions bot commented Mar 7, 2025 •

edited

Loading