[go: up one dir, main page]

Alcotezt: split Alcotezts into on Tezt test per Alcotest case

Context

As discussed in #5084 (closed) and #4726 (closed), this splits all Alcotezts testcases into one Tezt test per Alcotest case.

This depends on !8227 (merged) to avoid title collisions and so builds on that MR.

Manually testing the MR

Try dune exec src/lib_clic/main.exe -- --list before and after this MR. Notice all the new test cases!

Discussion

There were three main concern with splitting the tests:

  1. Their number would balloon
  2. As would the size of the checked in records
  3. tezt --list would become (more) unreadable

1. Number of test

$ tezt --list-tsv > tezts.tsv
$ wc -l tezts.tsv
5466 tezts.tsv
$ wc -l ../arvid@alcotezt-split/tezts.tsv
9313 ../arvid@alcotezt-split/tezts.tsv

By splitting tests, we go from 5466 to 9313 tests, a rough doubling. Not as bad as expected, and tolerable imo.

2. Records

I haven't measured the size of the records, but their size should scale linearly with number of tests. The checked in records weigh 1.5 MB, so we should go from 1.5 to 3MB. Tolerable, IMO. Since Tezt auto-auto balancing, we do send the records back and forth in the CI a bit more. However, they are first zipped to a measly 196KB. The zipped size of the records for splitted alcotezt should thus be around 400KB.

3. Long titles

On master, the line from --list is (roughly) 209 chars. With split tests, it goes up to 228. Worse, but not catastrophic.

On master

cat tezts.tsv | while read -r line; do echo "$(echo $line | wc -c):$line"; done | sort -t ':' -k 1 -n -r | head

The test producing the longest line in --list on master :

209:tezt/tests/sc_rollup.ml	Mumbai: wasm_2_0_0 - output exec (0, entrypoint: %default, eager: 5, intern, no_parameters_ty)	regression mumbai sc_rollup wasm_2_0_0 rollup_node outbox intern default no_parameters_ty

and its variations:

209:tezt/tests/sc_rollup.ml	Mumbai: wasm_2_0_0 - output exec (0, entrypoint: %default, eager: 5, extern, no_parameters_ty)	regression mumbai sc_rollup wasm_2_0_0 rollup_node outbox extern default no_parameters_ty
209:tezt/tests/sc_rollup.ml	Mumbai: wasm_2_0_0 - output exec (0, entrypoint: %default, eager: 0, intern, no_parameters_ty)	regression mumbai sc_rollup wasm_2_0_0 rollup_node outbox intern default no_parameters_ty
209:tezt/tests/sc_rollup.ml	Mumbai: wasm_2_0_0 - output exec (0, entrypoint: %default, eager: 0, extern, no_parameters_ty)	regression mumbai sc_rollup wasm_2_0_0 rollup_node outbox extern default no_parameters_ty
207:tezt/tests/sc_rollup.ml	Alpha: wasm_2_0_0 - output exec (0, entrypoint: %default, eager: 5, intern, no_parameters_ty)	regression alpha sc_rollup wasm_2_0_0 rollup_node outbox intern default no_parameters_ty
207:tezt/tests/sc_rollup.ml	Alpha: wasm_2_0_0 - output exec (0, entrypoint: %default, eager: 5, extern, no_parameters_ty)	regression alpha sc_rollup wasm_2_0_0 rollup_node outbox extern default no_parameters_ty

There is also:

209:tezt/tests/mockup.ml	Mumbai: (Mockup) Create mockup config show / init roundtrip [test_accounts,protocol_constants,show,init]	mumbai mockup client base_dir roundtrip init protocol_constants show test_accounts
209:tezt/tests/mockup.ml	Mumbai: (Mockup) Create mockup config show / init roundtrip [test_accounts,protocol_constants,init,show]	mumbai mockup client base_dir roundtrip init protocol_constants show test_accounts

which suffers from a similar problem. I don't have an immediate solution to shorten these titles, however, it is noteworthy that there is large amount of redundancy between the file, the title, and the tags. However, we currently require unique titles (unique file + title does not suffice). And I think it's hard to argue that the developer has misused tags here.

With splitted tests from this MR

The worst offenders are:

$ cat tezts.tsv | while read -r line; do echo "$(echo $line | wc -c):$line"; done | sort -t ':' -k 1 -n -r | head -n 50
228:protocol > integration	protocol > integration: 016-PtMumbai: liquidity baking (test liquidity baking does not shut off with toggle vote at 50% and baking 100 blocks longer than sunset level in previous protocols)	alcotezt quick
...
221:protocol > integration	protocol > integration: alpha: liquidity baking (test liquidity baking does not shut off with toggle vote at 50% and baking 100 blocks longer than sunset level in previous protocols)	alcotezt quick
217:Tezos_clic	Tezos_clic: auto-completion-parameters (non-terminal-seq: when first non-terminal-seq and its suffix are matched and args given, suggests all matching options in the next seq and its suffix)	alcotezt quick
213:protocol > integration	protocol > integration: 016-PtMumbai: liquidity baking (test liquidity baking originates three contracts when tzBTC does not exist and level indicates we might be on mainnet)	alcotezt quick
...
212:protocol > integration > consensus	protocol > integration > consensus: 016-PtMumbai: delegation (valid delegate registration: credit 1μꜩ, self delegation, debit 1μꜩ (switch with delegation))	alcotezt quick
....
211:protocol > integration > operations	protocol > integration > operations: 016-PtMumbai: sc rollup (Test that a player can't timeout another player before timeout period and related timeout value.)	alcotezt quick
210:Tezos_clic	Tezos_clic: auto-completion-parameters (non-terminal-seq: when first non-terminal-seq and its suffix are matched and no arg given, suggests all options in the next seq and its suffix)	alcotezt quick
210:synchronisation heuristic fuzzy	synchronisation heuristic fuzzy: synchronisation heuristic fuzzy (Shell.synchronisation_heuristic.equivalence-with-reference-implementation (threshold 8) (latency 100))	alcotezt
...
210:protocol > integration > consensus	protocol > integration > consensus: 016-PtMumbai: delegation (valid delegate registration: credit 1μꜩ, self delegation, debit 1μꜩ (init with delegation))	alcotezt quick
...
207:protocol > integration > operations	protocol > integration > operations: 016-PtMumbai: sc rollup (check that a commitment can be published after the inbox_level + challenge window is passed.)	alcotezt quick
206:protocol > integration	protocol > integration: alpha: liquidity baking (test liquidity baking originates three contracts when tzBTC does not exist and level indicates we might be on mainnet)	alcotezt quick

With this MR + !7965 (merged) (registering Alcotezts with __FILE__)

!7965 (merged) is gonna complicate matters slightly. Some of the tests are quite deeply nested which makes the longest line from tezt --list-tsv go up to 271 chars:

271:src/proto_016_PtMumbai/lib_protocol/test/integration/test_main.ml   protocol > integration: 016-PtMumbai: liquidity baking (test liquidity baking does not shut off with toggle vote at 50% and baking 100 blocks longer than sunset level in previous protocols)      alcotezt quick
271:src/proto_015_PtLimaPt/lib_protocol/test/integration/test_main.ml   protocol > integration: 015-PtLimaPt: liquidity baking (test liquidity baking does not shut off with toggle vote at 50% and baking 100 blocks longer than sunset level in previous protocols)      alcotezt quick
257:src/proto_alpha/lib_protocol/test/integration/test_main.ml  protocol > integration: alpha: liquidity baking (test liquidity baking does not shut off with toggle vote at 50% and baking 100 blocks longer than sunset level in previous protocols)     alcotezt quick
256:src/proto_016_PtMumbai/lib_protocol/test/integration/test_main.ml   protocol > integration: 016-PtMumbai: liquidity baking (test liquidity baking originates three contracts when tzBTC does not exist and level indicates we might be on mainnet)     alcotezt quick
256:src/proto_015_PtLimaPt/lib_protocol/test/integration/test_main.ml   protocol > integration: 015-PtLimaPt: liquidity baking (test liquidity baking originates three contracts when tzBTC does not exist and level indicates we might be on mainnet)     alcotezt quick

That makes for 1127 tests to shorten 😱 however, of those, 1037 are protocol tests, and there is ongoing work to shorten the titles (both manually in #5137 (closed) and automatically in !8109 (closed))

What's a reasonable limit

I think aiming for 70 or 80 chars, the "traditional" terminal width, is quite out of reach. So I'll say 190 chars, because that's the longest that my terminal will display without linebreaks. That makes for 125 lines to shorten. Sounds feasible, since many of them are variations of the same title.

Checklist

  • Document the interface of any function added or modified (see the coding guidelines)
  • Document any change to the user interface, including configuration parameters (see node configuration)
  • Provide automatic testing (see the testing guide).
  • For new features and bug fixes, add an item in the appropriate changelog (docs/protocols/alpha.rst for the protocol and the environment, CHANGES.rst at the root of the repository for everything else).
  • Select suitable reviewers using the Reviewers field below.
  • Select as Assignee the next person who should take action on that MR
Edited by Arvid Jakobsson

Merge request reports

Loading