Refactor how skipped tests are defined
維護者通常 3 天內回覆
還沒有人認領這個 Issue。
評估
研究方向
先閱讀 issue 中描述的 runtime.SkipIf 實作以及現有的 mscXXXX_test.go 組織方式。追蹤 build tags 如何選取測試,以及 runtime 如何偵測 homeserver,然後將該行為與提議的使用者設定和基於 t.Skipf 的執行方式進行比較。完成的標準是:設計支援容易發現的測試群組、使用者的明確選取、確定性的跳過,以及移除由 runtime 驅動的 blacklist 行為。
由索引模型根據 Issue 內容生成。
描述
Complement was designed to test the Matrix Specification. However, the spec isn't static and MSCs need tests too. We introduced the build tag system a long time ago to allow tests to opt-in to certain tests. They are defined like this:
//go:build msc3391
// +build msc3391
package tests
// ... rest of code
and used like this:
go test -tags "msc3083 msc3787 msc3874" ./...
The set of tests run is controlled by the user, which can be frustrating when they get out-of-sync: https://github.com/matrix-org/complement/issues/185
This wasn't the end of it though. We also have unique APIs specific to a homeserver, or sometimes some servers don't implement core features which are enabled by default. To fix this, we (ab)use the same build tag system. To opt-out of a test, they are defined like this:
//go:build !dendrite_blacklist
// +build !dendrite_blacklist
package tests
// ... rest of code
and used like this:
go test -tags "dendrite_blacklist" ./...
This was the status quo for a long time, until there was a need to blacklist some tests in the same file but not others. This introduced the runtime package, where you could skip a test in code, rather than conditional compilation. They are defined like this:
func TestRoomImageRoundtrip(t *testing.T) {
runtime.SkipIf(t, runtime.Dendrite)
// .. rest of test
}
A runtime knows what it is by the virtue of the blacklist tag. E.g running -tags "dendrite_blacklist" would skip this test, even if the homeservers were synapse! The code for this is:
//go:build dendrite_blacklist
// +build dendrite_blacklist
package runtime
import (
"context"
"time"
"github.com/docker/docker/client"
)
func init() {
Homeserver = Dendrite
}
This is clearly at odds with testing between heterogeneous homeservers. It's also been questioned if it is the test's job to say what can and cannot run with it. Surely it would be better if it were more tag-like, and the user can opt-in to these tests?
We also have some other ideas for conditional execution of tests based on /versions output: https://github.com/matrix-org/complement/issues/549 . Our forebear, sytest, would automatically skip some tests based on if an earlier test which has a special can_ property did not pass. This is problematic if the can_ test is flakey!
From these use cases, there are some properties we want the test execution API to have:
- granular but not too granular: MSC level is about right. Skipping entire files isn't great as the files are mostly arbitrary collections of tests.
- The user should specify which tests to execute. It's a bit of a smell that Complement dictates this currently via
runtime.SkipIf, and it means that this repository gets touched way too much just to remove these lines as servers get updated. - We don't want the set of tests executed to be nondeterminstic (dependent on a server response or a passing/failing previous test) as that is way too unstable and flakey.
A high-level approach here could be:
- One file per group of tests. Similar to
mscXXXX_test.gofiles. - Make test names part of the public testing API.
- Allow users to specify a set of groups / tests to run / not run.
This would allow us to remove runtime.SkipIf from the codebase, and runtime detection which is flakey and doesn't work with multi-HS support. It also removes the smell of having tests dictate what can/cannot run.
Proposal
- Define a configuration format for specifying the list of groups / test names to run. The list of valid groups/tests must be easily discoverable (autogen from code, like we do with env vars?)
- Read the configuration format and skip appropriately. Always use
t.Skipfinstead of conditional compilation? This would be beneficial as it is not uncommon for conditionally compiled code to break API wise from core (e.g alter a function signature in core, then patch up tests which whine, the mscXXXX files won't whine due to no build tags set by default in the IDE). - This implies some kind of test helper/executor that runs before every test, which isn't ideal as it starts to obfuscate what code runs when you hit test. Could just add it as boilerplate though?
- 主要語言
- Go
- 星號
- 101
- 分支
- 73
- 平均合併
- 4 天 21 小時
- 30 天內合併 PR
- 5
環境準備
- 沒有 Dockerfile 或 Docker Compose 檔案
- 有 Pull Request 範本
- 閱讀貢獻指南
從這裡開始
- 先讀完整個 Issue,再讀專案的貢獻指南。
- 在 Issue 下留言說明你要接手 —— 這能避免兩個人做同樣的事。
- Fork 儲存庫,在一個分支上完成修改。
- 送出 Pull Request,並在描述裡引用這個 Issue 編號。
matrix-org/complement 的其他 Issue
-
難度 2/5 1-3 小時 新手友好度 78/100
matrix-org/complement#900 · 1 則留言 ·
維護者通常 3 天內回覆
-
難度 5/5 一週以上 新手友好度 45/100
matrix-org/complement#938 ·
維護者通常 3 天內回覆
-
New tests for MSC4354 Sticky Events可能重新可做 @reivilibre 於 35 天前認領,目前沒有進行中的 PR。 未關閉
matrix-org/complement#925 · 已指派 1 人 ·
維護者通常 3 天內回覆
-
難度 3/5 1-2 天 新手友好度 52/100
matrix-org/complement#901 · 1 則留言 ·
維護者通常 3 天內回覆
-
難度 3/5 1-2 天 新手友好度 55/100
matrix-org/complement#899 ·
維護者通常 3 天內回覆
查看 matrix-org/complement 的全部 Issue
相似的 Issue
-
feat(gmail): make gmail get --format raw text output the exact message, like drafts get --format raw未關閉clawsweeper:needs-product-decision clawsweeper:no-new-fix-pr clawsweeper:source-repro impact:ux-friction issue-rating: 🦞 diamond lobster P3
難度 2/5 1-3 小時 新手友好度 78/100
openclaw/gogcli#1276 · 1 則留言 · 1 個 reaction ·
維護者通常 1 天內回覆
-
enhancement ready-for-agent
難度 2/5 1-3 小時 新手友好度 76/100
維護者通常 1 天內回覆
-
難度 2/5 1-3 小時 新手友好度 78/100
維護者通常 1 天內回覆
-
難度 2/5 1-3 小時 新手友好度 88/100
hiero-ledger/hiero-sdk-go#1886 ·
維護者通常 1 天內回覆
-
難度 2/5 1-3 小時 新手友好度 68/100
cloudevents/sdk-go#1346 ·