test: split input/output for planner test

[xxchan]

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

So that we can edit input files painlessly (e.g., no merge conflicts). The output files still contain the inputs for readability, and have exactly the same format as before.

(For reviewer: to confirm output is unchanged, run git diff --stat locally. GitHub is too slow 🤪)

If there are no such changes, it should be good

close #8557

The trick is similar to #9787, i.e., make planner tests snapshot testing.

Note: The new result check is simply assert eq the whole file, and produce the diff, instead check test cases one by one.

Limitations

• In theory the code can be further refactored, but I only made minimal changes to make it work.
• The error message produced by expect-test isn't optimal (see Panic message is too verbose for large result rust-analyzer/expect-test#39). But since the workflow is mostly just run dapt, and then use external diff tool to review it, it might be acceptable. Alternatively we can fork and improve expecte-test, or use insta instead.

Checklist For Contributors

Checklist For Reviewers

Documentation

• My PR DOES NOT contain user-facing changes.

Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

• Installation and deployment
• Connector (sources & sinks)
• SQL commands, functions, and operators
• RisingWave cluster configuration changes
• Other (please specify in the release note below)

Release note

[xxchan]

This is for reviewer to play with only

[codecov]

Codecov Report

Merging #9902 (e76d853) into main (7167524) will increase coverage by 0.05%.
The diff coverage is 88.96%.

@@            Coverage Diff             @@
##             main    #9902      +/-   ##
==========================================
+ Coverage   71.09%   71.14%   +0.05%     
==========================================
  Files        1250     1249       -1     
  Lines      209455   209310     -145     
==========================================
+ Hits       148904   148913       +9     
+ Misses      60551    60397     -154     

Flag	Coverage Δ
rust	71.14% <88.96%> (+0.05%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/frontend/planner_test/src/lib.rs	86.61% <88.43%> (+10.03%)	⬆️
src/frontend/planner_test/src/resolve_id.rs	100.00% <100.00%> (ø)

... and 8 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[chenzl25]

Generally LGTM. Maybe we need to tell developers that apply-planner-test no longer works and we only need do-apply-planner-test.

[st1page]

LGTM! So thanks.

[st1page]

just to confirm, what will happen when some cases panicked? And how a developer can not which case panic in the new framework? In other words, we might need some logic to replace this code.

risingwave/src/frontend/planner_test/src/bin/apply.rs

Lines 26 to 39 in 68140f3

	async fn main() -> Result<()> {
	std::panic::set_hook(Box::new(move |e| {
	let backtrace = Backtrace::new();
	println!(
	"{}{}{}{}{}\n{:?}\n{e}",
	style("ERROR: ").red().bold(),
	style("apply-planner-test").yellow(),
	style(" panicked! Try ").red().bold(),
	style("run-planner-test --no-fail-fast").yellow(),
	style(" to find which test case panicked.").red().bold(),
	backtrace,
	);
	std::process::abort();
	}));

[TennyZhuang]

LGTM

[xxchan]

just to confirm, what will happen when some cases panicked? And how a developer can not which case panic in the new framework? In other words, we might need some logic to replace this code.

We don't need to handle that now. We used to do it because apply is a separate binary. And it tells you to run run-planner-test (nextest) for debugging. Now we only run nextest which catches panic for us.

[stdrc]

LGTM