-
Notifications
You must be signed in to change notification settings - Fork 3.4k
rebaseline_tests.py: Add --new-branch and --clear. NFC #23192
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
Conversation
c38e300
to
72f59d3
Compare
parser = argparse.ArgumentParser() | ||
parser.add_argument('-s', '--skip-tests', action='store_true', help="don't actually run the tests, just analyze the existing results") | ||
parser.add_argument('-b', '--new-branch', action='store_true', help='create a new branch containing the updates') | ||
parser.add_argument('-c', '--clear-cache', action='store_true', help='clear the cache before rebaselining (useful when working with llvm changes)') |
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.
Should this be the default perhaps?
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.
I think most of the time this script will be used to update expectations in response for local emscripten changes (i.e. minor changes to JS libraries) not upstream llvm change. At least for me I often run rebases without clearing the cache when I know llvm has not changed.
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.
It also makes it much slower if you clear the cache first, and when this script runs in CI it would never be needed (since CI libs are always built using tot anyway).
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.
Fair enough. lgtm either way.
72f59d3
to
a14a73d
Compare
No description provided.