Tracking PR: Testsuite rework

[eva-cosma]

Pull Request Overview

This pull request tracks all outstanding PRs regarding to the testsuite rework, their purpose, and other relevant information.

Overview of mentioned PRs (so you can see their names also)

• Motivation: Add RwLock and concurrency enabled LinearMemory #136
• TS Preview iteration 1: feat: Testsuite Preview github action #126
• Panic fix: fix: Catch Unwind in direct invoke directive #143
• Fix + Testsuite rework: Dev/testsuite rework 2 #146
• Fix + Rework + TS Preview iteration 2: Dev/new testsuite preview #147
• Fix + Rework + TS Preview iteration 2 (without crossing fork boundaries): Please ignore :) #149

Motivation

A bug within the testsuite runner was detected not so long ago in PR #136 . Some direct invoke directive caused the testsuite runner to crash due to an uncaught panic in the handling of those directives. This was a double-edged sword. Firstly, it indicated a bug in the implementation, however the panic should have been caught and reported by the runner (and thus the PR should not have failed but we would also not notice the error).

This is a tricky approach. On one hand, we want to see all possible bugs caught by the testsuite, but on the other hand we have so many non-working tests that it is close to impossible to determine which are caused by features not being yet implement and which are determined by a faulty implementation.

A meet-in-the-middle solution would be to report all changes to the testsuite results in a PR. This would catch previously passing .wast files becoming failing or with a script error (previously named compilation error). This would not catch a script error's cause being different¹.

In this entire process, a rework to the code itself was made.

Relevant PRs

The first relevant PR is #126 . This added the first draft to a github action which would print out the changes.

After some time, the bug in #136 happened, and as a response PR #143.

Afterwards, I decided to rework the code for the testsuite runner to enable more changes (and in general have the code be cleaner). The rework was done in PR #146 and contains the changes in PR #143.

After that, I've reworked the testsuite preview (#126) to take advantage of the rework, and in general made it more robust. There are 2 PRs relevant to this #147 that merges from george-cosma fork and #149 that merges from this repo. The reasons I mention this is that #147 kept failing to post the comment due to permission issues. This has been fixed, but due to security reasons Github will not run that action without it being first merged into main (or the target branch of the PR, but we usually merge into main)

This project might open up for more people in the future, and we want our CI actions to work even if they come from across forks.

What's left to do?

First, the commit history needs to be cleaned up. Unfortunately, testing actions that change PR contents are nearly impossible to test locally. Secondly, nix flake check needs to pass.

Thirdly, we might want to change how the difference checker algorithm works if a change in error message is detected. However, I would leave this feature to another PR.

Footnotes

1. A feature could be implemented to check if the error message is different, at best ↩

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[florianhartung]

closed by #163