Skip to content

Commit

Permalink
CI: Check that test expectations are up-to-date (#23201)
Browse files Browse the repository at this point in the history
This test will fail if the expectations are out-of-date on the main
branch. This can happen due to either:

1. Somebody forgetting to update the expectations when they land an
emscripten change
2. llvm or binaryen changes that rolled in on the auto-roller.

For now I propose that we don't make this new check "Required" and we
consider this the first step towards making these updates easy and
automatic.

I made this a github action rather than using CiricleCI since I
anticipate using github automation here in the future (for example to
suggest updates the current PR).
  • Loading branch information
sbc100 authored Dec 17, 2024
1 parent 84de057 commit f1639a3
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 43 deletions.
27 changes: 0 additions & 27 deletions .github/workflows/archive.yml

This file was deleted.

66 changes: 66 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
name: CI

on:
create:
tags:
push:
branches:
- main
pull_request:

permissions:
contents: read

jobs:
archive:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@8ade135a41bc03ea155e62e844d188df1ea18608 # v4.1.0
- name: make dist
run: |
make dist
version=`cat emscripten-version.txt | sed s/\"//g`
echo "VERSION=$version" >> $GITHUB_ENV
- uses: actions/upload-artifact@0b7f8abb1508181956e8e162db84b466c27e18ce # v3.1.2
with:
name: emscripten-${{ env.VERSION }}
path: emscripten-${{ env.VERSION }}.tar.bz2

check-expectations:
runs-on: ubuntu-latest
steps:
- name: Checkout repo
uses: actions/checkout@v4
with:
fetch-depth: 0 # We want access to other branches, specifically `main`
- name: pip install
run: |
which python3
python3 --version
python3 -m pip install -r requirements-dev.txt
- name: Install emsdk
run: |
EM_CONFIG=$HOME/emsdk/.emscripten
echo $EM_CONFIG
echo "EM_CONFIG=$EM_CONFIG" >> $GITHUB_ENV
curl -# -L -o ~/emsdk-main.tar.gz https://github.com/emscripten-core/emsdk/archive/main.tar.gz
tar -C ~ -xf ~/emsdk-main.tar.gz
mv ~/emsdk-main ~/emsdk
cd ~/emsdk
./emsdk install tot
./emsdk activate tot
echo "JS_ENGINES = [NODE_JS]" >> $EM_CONFIG
echo "final config:"
cat $EM_CONFIG
- name: Check test expectations on main
run: |
git checkout origin/main
# Hack to honor changes to rebaseline_tests.py in the current PR
git checkout - ./tools/maint/rebaseline_tests.py
./bootstrap
if ! ./tools/maint/rebaseline_tests.py --check-only; then
echo "Test expectations are out-of-date on the main branch."
echo "You can run `./tools/maint/rebaseline_tests.py --new-branch`"
echo "and use it to create a seperate PR."
exit 1
fi
46 changes: 30 additions & 16 deletions tools/maint/rebaseline_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
'browser.test_small_js_flags',
'other.test_INCOMING_MODULE_JS_API',
'other.*code_size*',
'other.*codesize*'
'other.*codesize*',
'skip:other.test_jspi_code_size',
]


Expand Down Expand Up @@ -63,33 +64,43 @@ def process_changed_file(filename):

def main(argv):
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)')
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)')
parser.add_argument('-n', '--check-only', dest='check_only', action='store_true', help='Return non-zero if test expectations are out of date, and skip creating a git commit')
args = parser.parse_args()

if args.clear_cache:
run(['emcc', '--clear-cache'])

if not args.skip_tests:
if run(['git', 'status', '-uno', '--porcelain']).strip():
if not args.check_only and run(['git', 'status', '-uno', '--porcelain']).strip():
print('tree is not clean')
return 1

subprocess.check_call(['test/runner', '--rebaseline', '--browser=0'] + TESTS, cwd=root_dir)

if not run(['git', 'status', '-uno', '--porcelain']):
print('no updates found')
return 1
if not run(['git', 'status', '-uno', '--porcelain', 'test']):
print('test expectations are up-to-date')
return 0

output = run(['git', 'status', '-uno', '--porcelain'])
filenames = []
for line in output.splitlines():
status, filename = line.strip().rsplit(' ', 1)
if filename.startswith('test/'):
filename = line.strip().rsplit(' ', 1)[1]
if filename.startswith('test'):
filenames.append(filename)

commit_message = f'''
if args.check_only:
message = f'''Test expectations are out-of-date
The following ({len(filenames)}) test expectation files were updated by
running the tests with `--rebaseline`:
```
'''
else:
message = f'''
Automatic rebaseline of codesize expectations. NFC
This is an automatic change generated by tools/maint/rebaseline_tests.py.
Expand All @@ -101,18 +112,21 @@ def main(argv):
'''

for file in filenames:
commit_message += process_changed_file(file)
message += process_changed_file(file)

commit_message += f'\nAverage change: {statistics.mean(all_deltas):+.2f}% ({min(all_deltas):+.2f}% - {max(all_deltas):+.2f}%)\n'
message += f'\nAverage change: {statistics.mean(all_deltas):+.2f}% ({min(all_deltas):+.2f}% - {max(all_deltas):+.2f}%)\n'

commit_message += '```\n'
message += '```\n'

print(message)
if args.check_only:
return 1

if args.new_branch:
run(['git', 'checkout', '-b', 'rebaseline_tests'])
run(['git', 'add', '-u', '.'])
run(['git', 'commit', '-F', '-'], input=commit_message)
run(['git', 'commit', '-F', '-'], input=message)

print(commit_message)
return 0


Expand Down

0 comments on commit f1639a3

Please sign in to comment.