emscripten-core / emscripten-core/emscripten

Create a script + github action for rebaselining tests

Open
#23,146 4 comments 0 reactions 1 assignee Claimed by @sbc100 View on GitHub
Dominant language
C++
Stars
27.6k
Forks
3.6k
Avg merge
1d 1h
Merged PRs (30d)
105

Description

We have a bunch of codesize tests that need to be rebaselined. This need can occur under 2 different circumstances.

1. A llvm or binaryen commit rolled in that changes the generated code.
2. An emscripten PR causes changes to the generated code.

I think it would be good if we can avoid mixing (1) and (2). These means that the main branch should always be updated to include changes from llvm/binaryen before the changes for specific PR are landed (as part of that PR).

For (1) procedure I've been using looks something like this:

```
$ git checkout -b main upstream/main
$ ./emcc --clear-cache
$ test/runner other.*code_size* other.*codesize* --rebase
$ git add -u .
$ git commit -m "Rebaseline codesize expectations. NFC"
$ git show # check everything looks reasonable
$ git push upsteam # direct push to upstream, no PR
```

There are several problems with this (as pointed out in https://github.com/emscripten-core/emscripten/pull/23126#discussion_r1879129835).

1. Its a lot of separate steps
2. It involves pushing directly to repo, without any review
3. Its hard to see (using `git diff`) exactly what effect the changes have.

I propose adding a new script that performs most of the step above that will have the following advantages:

1. It can analyze the results and present a human readable report (e.g. `test xxx regressed by 10 bytes (0.1%)`).
2. It can automatically create a PR with the report in the description.
3. It can be turned into a github action so it can be run on a remote machine (not a users machine where llvm and binaries version might not be exactly at `tot`).

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.