Re: [RFC PATCH] drm/ci: add helper script update-xfails.py
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Hi Helen,
On Wed, 2023-09-27 at 19:28 -0300, Helen Koike wrote:
> > > +def get_unit_test_name_and_results(unit_test):
> > > + if "Artifact results/failures.csv not found" in unit_test:
> > > + return None, None
> > > + unit_test_name, unit_test_result =
> > > unit_test.strip().split(",")
> > > + return unit_test_name, unit_test_result
> >
> > Suggestion: it is not managing empty lines or comments. By now,
> > there
> > aren't, but they could be found.
>
> Indeed.
Just add a new if statement to discard if the strings start with # or
strip the line and check the length. Perhaps we can think of other
assertions to sanitise the string.
> > Suggestion: Sometimes tests fails with different status ("Fail" to
> > "Crash" for example) and the expectations should be updated with
> > the
> > newer status.
>
> The status is only present in the fails and not in the flakes list,
> so I
> update it with add_unit_test_or_update_result_to_fails_if_present()
> function below, make sense?
Absolutely, sorry that I didn't see this was a process included in the
last if statement. If it is present in the fails' file (that includes
the test name and its state) you do exactly what's necessary: add if
not present, update if it was already in the file.
>
Regards,
- --
Sergi Blanch Torné
Senior Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718
-BEGIN PGP SIGNATURE-
iHUEARYIAB0WIQQwWRK68l+taJfhwqAto5bHyTm9RwUCZRUnAgAKCRAto5bHyTm9
R53NAP9T2OCiwbnEjv+H0CQg/eK1xGe7yS/3cqjaPFRvvZPp1wD/V1H9NuhpRR6M
8+QZgbsS/swSPdwYABtcz+75CKpuJwo=
=XRRO
-END PGP SIGNATURE-
Re: [RFC PATCH] drm/ci: add helper script update-xfails.py
Hi Sergi,
Thanks for your comments.
On 27/09/2023 05:58, Sergi Blanch Torne wrote:
Hi Helen,
On Mon, 2023-09-25 at 16:55 -0300, Helen Koike wrote:
Hello,
This script is being very handy for me, so I suppose it could be
handy
to others, since I'm publishing it in the xfails folder.
Let me know your thoughts.
I have two suggestions and one comment. Before that, I would like to
highlight the importance of tools like that to help with repetitive
tedious work, it is great to have this script.
+def get_xfails_file_path(canonical_name, suffix):
+name = canonical_name.replace(":", "-")
+script_dir = os.path.dirname(os.path.abspath(__file__))
+return os.path.join(script_dir, f"{name}-{suffix}.txt")
I appreciate the correspondence between job name and expectation file
names.
+def get_unit_test_name_and_results(unit_test):
+if "Artifact results/failures.csv not found" in unit_test:
+return None, None
+unit_test_name, unit_test_result = unit_test.strip().split(",")
+return unit_test_name, unit_test_result
Suggestion: it is not managing empty lines or comments. By now, there
aren't, but they could be found.
Indeed.
+def main(namespace, project, pipeline_id):
+xfails = (
+Collate(namespace=namespace, project=project)
+.from_pipeline(pipeline_id)
+.get_artifact("results/failures.csv")
+)
+
+split_unit_test_from_collate(xfails)
+
+for job_name in xfails.keys():
+canonical_name = get_canonical_name(job_name)
+fails_txt_path = get_xfails_file_path(canonical_name,
"fails")
+flakes_txt_path = get_xfails_file_path(canonical_name,
"flakes")
+
+fails_txt = read_file(fails_txt_path)
+flakes_txt = read_file(flakes_txt_path)
+
+for job_id in xfails[job_name].keys():
+for unit_test in xfails[job_name][job_id]:
+unit_test_name, unit_test_result =
get_unit_test_name_and_results(unit_test)
+
+if not unit_test_name:
+continue
+
+if is_test_present_on_file(flakes_txt,
unit_test_name):
+remove_unit_test_if_present(flakes_txt,
unit_test_name, flakes_txt_path)
+print("WARNING: unit test is on flakes list but
a job failed due to it, "
+ "is your tree up to date?",
+ unit_test_name, "DROPPED FROM",
os.path.basename(flakes_txt_path))
+
+if unit_test_result == "UnexpectedPass":
+remove_unit_test_if_present(fails_txt,
unit_test_name, fails_txt_path)
+# flake result
+if not
is_unit_test_present_in_other_jobs(unit_test, xfails[job_name]):
+add_unit_test_if_not_present(flakes_txt,
unit_test_name, flakes_txt_path)
+continue
Suggestion: Sometimes tests fails with different status ("Fail" to
"Crash" for example) and the expectations should be updated with the
newer status.
The status is only present in the fails and not in the flakes list, so I
update it with add_unit_test_or_update_result_to_fails_if_present()
function below, make sense?
Regards,
Helen
+
+# flake result
+if not is_unit_test_present_in_other_jobs(unit_test,
xfails[job_name]):
+add_unit_test_if_not_present(flakes_txt,
unit_test_name, flakes_txt_path)
+continue
+
+# consistent result
+
add_unit_test_or_update_result_to_fails_if_present(fails_txt,
unit_test,
+
fails_txt_path)
+
+save_file(fails_txt, fails_txt_path)
+save_file(flakes_txt, flakes_txt_path)
Regards,
Re: [RFC PATCH] drm/ci: add helper script update-xfails.py
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256
Hi Helen,
On Mon, 2023-09-25 at 16:55 -0300, Helen Koike wrote:
> Hello,
>
> This script is being very handy for me, so I suppose it could be
> handy
> to others, since I'm publishing it in the xfails folder.
>
> Let me know your thoughts.
I have two suggestions and one comment. Before that, I would like to
highlight the importance of tools like that to help with repetitive
tedious work, it is great to have this script.
> +def get_xfails_file_path(canonical_name, suffix):
> + name = canonical_name.replace(":", "-")
> + script_dir = os.path.dirname(os.path.abspath(__file__))
> + return os.path.join(script_dir, f"{name}-{suffix}.txt")
I appreciate the correspondence between job name and expectation file
names.
> +def get_unit_test_name_and_results(unit_test):
> + if "Artifact results/failures.csv not found" in unit_test:
> + return None, None
> + unit_test_name, unit_test_result = unit_test.strip().split(",")
> + return unit_test_name, unit_test_result
Suggestion: it is not managing empty lines or comments. By now, there
aren't, but they could be found.
> +def main(namespace, project, pipeline_id):
> + xfails = (
> + Collate(namespace=namespace, project=project)
> + .from_pipeline(pipeline_id)
> + .get_artifact("results/failures.csv")
> + )
> +
> + split_unit_test_from_collate(xfails)
> +
> + for job_name in xfails.keys():
> + canonical_name = get_canonical_name(job_name)
> + fails_txt_path = get_xfails_file_path(canonical_name,
> "fails")
> + flakes_txt_path = get_xfails_file_path(canonical_name,
> "flakes")
> +
> + fails_txt = read_file(fails_txt_path)
> + flakes_txt = read_file(flakes_txt_path)
> +
> + for job_id in xfails[job_name].keys():
> + for unit_test in xfails[job_name][job_id]:
> + unit_test_name, unit_test_result =
> get_unit_test_name_and_results(unit_test)
> +
> + if not unit_test_name:
> + continue
> +
> + if is_test_present_on_file(flakes_txt,
> unit_test_name):
> + remove_unit_test_if_present(flakes_txt,
> unit_test_name, flakes_txt_path)
> + print("WARNING: unit test is on flakes list but
> a job failed due to it, "
> + "is your tree up to date?",
> + unit_test_name, "DROPPED FROM",
> os.path.basename(flakes_txt_path))
> +
> + if unit_test_result == "UnexpectedPass":
> + remove_unit_test_if_present(fails_txt,
> unit_test_name, fails_txt_path)
> + # flake result
> + if not
> is_unit_test_present_in_other_jobs(unit_test, xfails[job_name]):
> + add_unit_test_if_not_present(flakes_txt,
> unit_test_name, flakes_txt_path)
> + continue
Suggestion: Sometimes tests fails with different status ("Fail" to
"Crash" for example) and the expectations should be updated with the
newer status.
> +
> + # flake result
> + if not is_unit_test_present_in_other_jobs(unit_test,
> xfails[job_name]):
> + add_unit_test_if_not_present(flakes_txt,
> unit_test_name, flakes_txt_path)
> + continue
> +
> + # consistent result
> +
> add_unit_test_or_update_result_to_fails_if_present(fails_txt,
> unit_test,
> +
> fails_txt_path)
> +
> + save_file(fails_txt, fails_txt_path)
> + save_file(flakes_txt, flakes_txt_path)
Regards,
- --
Sergi Blanch Torné
Senior Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718
-BEGIN PGP SIGNATURE-
iHUEARYIAB0WIQQwWRK68l+taJfhwqAto5bHyTm9RwUCZRPuvgAKCRAto5bHyTm9
R7wIAP4hr5ddBZ9+3R4n2KJA5DOc6JE1oRjabB7A2pkZFl1BxwEAyX83yMaqJE8T
RXrhZ3oyQbUIyCbZhLNOP6OiZ6bchQc=
=Pr1y
-END PGP SIGNATURE-
[RFC PATCH] drm/ci: add helper script update-xfails.py
Add helper script that given a gitlab pipeline url, analise which are
the failures and flakes and update the xfails folder accordingly.
Example:
Trigger a pipeline in gitlab infrastructure, than re-try a few jobs more
than once (so we can have data if failues are consistent across jobs
with the same name or if they are flakes) and execute:
update-xfails.py
https://gitlab.freedesktop.org/helen.fornazier/linux/-/pipelines/970661
git diff should show you that it updated files in xfails folder.
Signed-off-by: Helen Koike
---
Hello,
This script is being very handy for me, so I suppose it could be handy
to others, since I'm publishing it in the xfails folder.
Let me know your thoughts.
Regards,
Helen
---
drivers/gpu/drm/ci/xfails/requirements.txt | 16 ++
drivers/gpu/drm/ci/xfails/update-xfails.py | 161 +
2 files changed, 177 insertions(+)
create mode 100644 drivers/gpu/drm/ci/xfails/requirements.txt
create mode 100755 drivers/gpu/drm/ci/xfails/update-xfails.py
diff --git a/drivers/gpu/drm/ci/xfails/requirements.txt
b/drivers/gpu/drm/ci/xfails/requirements.txt
new file mode 100644
index ..26255fb6d6b8
--- /dev/null
+++ b/drivers/gpu/drm/ci/xfails/requirements.txt
@@ -0,0 +1,16 @@
+git+https://gitlab.freedesktop.org/gfx-ci/ci-collate@4a5bb602855f2bd4fc1ecf43db19e84a906f4258
+
+# ci-collate dependencies
+certifi==2023.7.22
+charset-normalizer==3.2.0
+idna==3.4
+pip==23.2.1
+python-gitlab==3.15.0
+requests==2.31.0
+requests-toolbelt==1.0.0
+ruamel.yaml==0.17.32
+ruamel.yaml.clib==0.2.7
+setuptools==68.0.0
+tenacity==8.2.3
+urllib3==2.0.4
+wheel==0.41.1
diff --git a/drivers/gpu/drm/ci/xfails/update-xfails.py
b/drivers/gpu/drm/ci/xfails/update-xfails.py
new file mode 100755
index ..f14f8ea78de7
--- /dev/null
+++ b/drivers/gpu/drm/ci/xfails/update-xfails.py
@@ -0,0 +1,161 @@
+#!/usr/bin/env python3
+
+import argparse
+import os
+import re
+from glcollate import Collate
+from urllib.parse import urlparse
+
+
+def get_canonical_name(job_name):
+return re.split(r" \d+/\d+", job_name)[0]
+
+
+def get_xfails_file_path(canonical_name, suffix):
+name = canonical_name.replace(":", "-")
+script_dir = os.path.dirname(os.path.abspath(__file__))
+return os.path.join(script_dir, f"{name}-{suffix}.txt")
+
+
+def get_unit_test_name_and_results(unit_test):
+if "Artifact results/failures.csv not found" in unit_test:
+return None, None
+unit_test_name, unit_test_result = unit_test.strip().split(",")
+return unit_test_name, unit_test_result
+
+
+def read_file(file_path):
+try:
+with open(file_path, "r") as file:
+f = file.readlines()
+if len(f):
+f[-1] = f[-1].strip() + "\n"
+return f
+except FileNotFoundError:
+return []
+
+
+def save_file(content, file_path):
+# delete file is content is empty
+if not content or not any(content):
+if os.path.exists(file_path):
+os.remove(file_path)
+return
+
+content.sort()
+with open(file_path, "w") as file:
+file.writelines(content)
+
+
+def is_test_present_on_file(file_content, unit_test_name):
+return any(unit_test_name in line for line in file_content)
+
+
+def is_unit_test_present_in_other_jobs(unit_test, job_ids):
+return all(unit_test in job_ids[job_id] for job_id in job_ids)
+
+
+def remove_unit_test_if_present(lines, unit_test_name, file_name):
+if not is_test_present_on_file(lines, unit_test_name):
+return
+lines[:] = [line for line in lines if unit_test_name not in line]
+print(os.path.basename(file_name), ": REMOVED", unit_test_name)
+
+
+def add_unit_test_if_not_present(lines, unit_test_name, file_name):
+if all(unit_test_name not in line for line in lines):
+lines.append(unit_test_name + "\n")
+print(os.path.basename(file_name), ": ADDED", unit_test_name)
+
+
+def update_unit_test_result_in_fails_txt(fails_txt, unit_test, file_name):
+unit_test_name, unit_test_result =
get_unit_test_name_and_results(unit_test)
+for i, line in enumerate(fails_txt):
+if unit_test_name in line:
+_, current_result = get_unit_test_name_and_results(line)
+fails_txt[i] = unit_test + "\n"
+print(os.path.basename(file_name), ": UPDATED", unit_test,
+ "FROM", current_result, "TO", unit_test_result)
+return
+
+
+def add_unit_test_or_update_result_to_fails_if_present(fails_txt, unit_test,
fails_txt_path):
+unit_test_name, _ = get_unit_test_name_and_results(unit_test)
+if not is_test_present_on_file(fails_txt, unit_test_name):
+add_unit_test_if_not_present(fails_txt, unit_test, fails_txt_path)
+# if it is present but not with the same result
+elif not is_test_present_on_file(fails_txt, unit_test):
+update_unit_test_result_in_fails_txt(fails_txt, unit_test,
fails_txt_path)
+
+
+def split_unit_test_from_collate(xfails):
+for job_n
