Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: use #guard_msgs in run tests #4175

Merged
merged 3 commits into from
May 16, 2024
Merged

Conversation

semorrison
Copy link
Collaborator

Many of our tests in tests/lean/run/ produce output from #eval (or #check) statements, that is then ignored.

This PR tries to capture all the useful output using #guard_msgs. I've only done a cursory check that the output is still sane --- there is a chance that some "unchecked" tests have already accumulated regressions and this just cements them!

In the other direction, I did identify two rotten tests:

  • a minor one in setStructInstNotation.lean, where a comment says Set Nat, but #check actually prints ?_. Weird?
  • CompilerProbe.lean is generating empty output, apparently indicating that something is broken, but I don't know the signficance of this file.

In any case, I'll ask about these elsewhere.

(This started by noticing that a recent grind test file had an untested trace_state, and then got carried away.)

@nomeata
Copy link
Contributor

nomeata commented May 15, 2024

Thanks a lot!

Once that is taken care of, I’d like to propose that a run test with any output is considered failing, to avoid mistakes.

(And then we could actually remove special handling of run and treat an absent .expected file as indicating that the expected output is empty. Maybe with an opt-in escape hatch for “really ignore this output, even though I cannot use #guard_msg for it.)

@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label May 15, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 15, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 15, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label May 15, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented May 15, 2024

Mathlib CI status (docs):

@semorrison
Copy link
Collaborator Author

Once that is taken care of, I’d like to propose that a run test with any output is considered failing

We're not quite there yet. There are many #check statements that I though it was pointless to wrap in #guard_msgs. The output here is irrelevant, the point in the test file is that it typechecks.

There are also many cases where a test either #prints a large statement, or looks a IR output, or long simp/defeq/etc trace output... So there is more thought and cleanup required before we can silence tests.

I guess the intermediate solution is to split run into the quiet ones and the noisy ones.

@semorrison semorrison enabled auto-merge May 16, 2024 00:23
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 16, 2024 00:37 Inactive
@semorrison semorrison added this pull request to the merge queue May 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request May 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request May 16, 2024
Merged via the queue into master with commit 3a457e6 May 16, 2024
13 checks passed
@nomeata
Copy link
Contributor

nomeata commented May 16, 2024

I'd the point of the check command is not it's output, you can make that explicit using #guard_msgs() in, can't you?

@digama0
Copy link
Collaborator

digama0 commented May 16, 2024

I'd the point of the check command is not it's output, you can make that explicit using #guard_msgs() in, can't you?

I think you mean #guard_msgs(drop info)

@nomeata
Copy link
Contributor

nomeata commented May 16, 2024

Some variant thereof, yes :-D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants