-
Notifications
You must be signed in to change notification settings - Fork 604
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
tests: add feature tagging workflow #15148
base: master
Are you sure you want to change the base?
tests: add feature tagging workflow #15148
Conversation
Fri Mar 21 11:00:02 UTC 2025 Spread tests skipped |
3d46364
to
34ed733
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #15148 +/- ##
==========================================
+ Coverage 78.07% 78.10% +0.02%
==========================================
Files 1182 1191 +9
Lines 157743 159148 +1405
==========================================
+ Hits 123154 124297 +1143
- Misses 26943 27118 +175
- Partials 7646 7733 +87
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
34ed733
to
44ba42a
Compare
…sing '\' to using '--' to reflect changes in debug tagging PR
c4ff4f0
to
ba0efa9
Compare
ba0efa9
to
75369e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass. I'm kind of -1 on the python that looks more like perl in the sense that everything is a dict. Can we avoid that and actually use objects of some type (typedict is a hack but passable). If loading json is a problem we can sync to find a solution.
@@ -0,0 +1,18 @@ | |||
rules: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please start with a comment that links to an explanation of what this file is for and what reads it?
tests/lib/compose-features.py
Outdated
for system in systems: | ||
composed = compose_system(dir=args.dir, system=system, | ||
failed_tests=args.failed_tests, env_variables=args.env_variables) | ||
system = "_".join(system.split(':')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can just replace:
system = "_".join(system.split(':')) | |
system = system.replace(":", "_") |
tests/lib/compose-features.py
Outdated
failed_tests=args.failed_tests, env_variables=args.env_variables) | ||
system = "_".join(system.split(':')) | ||
with open(os.path.join(args.output, system + attempt + '.json'), 'w') as f: | ||
f.write(json.dumps(composed)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you can avoid the intermediate:
f.write(json.dumps(composed)) | |
json.dump(composed, f) |
tests/lib/compose-features.py
Outdated
|
||
|
||
if __name__ == '__main__': | ||
parser = argparse.ArgumentParser(description=""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move the description variable above main to make this format with less indent?
tests/lib/compose-features.py
Outdated
def run_attempt_type(value): | ||
if value is not int or int(value) <= 0: | ||
raise argparse.ArgumentTypeError( | ||
"%s is invalid. Run attempts are integers and start at 1" % value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
% is somewhat discouraged, perhaps, f-string or just plain .format?
tests/lib/compose-features.py
Outdated
'_1') and any(rerun for rerun in reruns_no_ext if rerun.startswith(file[:-2]))] | ||
reruns_no_ext.sort(key=lambda x: int(x.split('_')[-1])) | ||
for rerun in reruns_no_ext: | ||
beginning = '_'.join(rerun.split('_')[:-1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wants to be a helper but I'm not sure how to call it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
tests/lib/compose-features.py
Outdated
''' | ||
files = [f for f in os.listdir( | ||
dir) if os.path.isfile(os.path.join(dir, f))] | ||
systems = [":".join(file.split(':')[:2]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be a helper that returns a namedtuple and the exepression here grabs the system attribute?
Can you make the whole thing a set comprehension please?
tests/lib/compose-features.py
Outdated
return original_name, suite_name, test_name, variant_name | ||
|
||
|
||
def _compose_test(dir: str, file: str, failed_tests: str) -> dict: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You know you want to use typed dict?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
run-spread
Outdated
if [ -z "$SPREAD_TAG_FEATURES" ]; then | ||
SPREAD_USE_PREBUILT_SNAPD_SNAP=true exec spread "$@" | ||
else | ||
WRITE_DIR="/tmp/features" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, can we use a temp dir?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this would be for personal use, I thought tmp would be the best option.
run-spread
Outdated
done | ||
|
||
./tests/lib/compose-features.py \ | ||
--dir ${WRITE_DIR}/composed-feature-tags \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we shellcheck the file and shfmt the text later?
d4ad170
to
56daeb9
Compare
56daeb9
to
49a5046
Compare
Dependent on feature tag logging in #15091
This adds the ability to generate feature tags for a spread run in two ways:
SPREAD_TAG_FEATURES
setBoth methods permit automatically rerunning spread and consolidating the ending results into system-by-system json files of features. In the workflow, the results are uploaded as an artifact, though this is a temporary measure since in the future the tags will instead be loaded into a DB elsewhere.
Both the run-spread additions and the workflow contain a rerunning mechanism to automatically rerun spread with the failed tests a maximum number of times. Once the workflow/run-spread script has finished all spread runs, including reruns, it will consolidate the feature tagging results into a single final json file for each system. It consolidates results by creating a final version with all tests run where more recent individual test results substitute the less recent.