-
Notifications
You must be signed in to change notification settings - Fork 451
add dune describe tests
subcommand
#12545
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
base: main
Are you sure you want to change the base?
Conversation
Looks like a good start. I think you should start with a human readable format before moving on to machine readable formats though. Following that, the most important extension to this work would be to support cram and ppx_expect tests. Those are at least as important as tests defined using the "test" stanza. |
bin/describe/describe_tests.ml
Outdated
let to_dyn { names; source_dir; package; deps; enabled; action; locks } = | ||
let open Dyn in | ||
record | ||
[ "names", list string names |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the fact that multiple tests can be define in one stanza isn't something that the describe command should necessarily display. A separate test entry for each test in the same stanza seems easier for tools/users to make use of.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I went with your suggestion. I updated the implementation so that each test in a multi-test stanza is listed individually in the dune describe tests output.
Implement a new subcommand to list all test stanzas (unit tests and CRAM) with metadata including name, location, target, package, and enabled status. - Support both Tests.T and Cram_stanza.T stanzas - Generate actionable targets: `.exe` for unit tests, `@dir/runtest` for CRAM - Handle multiple tests per stanza as separate entries - Expose target and location fields for IDE integration - Add comprehensive test suite in describe-tests.t Fixes ocaml#12030 Signed-off-by: Rodrigue LEITAO--PEREIRA DIAS <[email protected]>
ac3d954
to
bc30130
Compare
Since implementing Cram stanzas is straightforward, I included Cram support in the new revision. Supporting ppx_expect tests is less direct, as they are not defined in configuration files. Handling them would likely require parsing the test files themselves ? |
Right, it's a bit more difficult. I don't think you need to parse the tests themselves (although that would certainly be a good thing too). It's enough to just rely on the rules to discover the test files. That would already be enough granularity. What do you think about dropping the wrap sexp around the output like PS: CI is pointing out some minor issues |
Objective
Add a new
dune describe tests
subcommand to display information about test and cram stanzas defined in the current project.This information includes metadata such as:
enabled_if
conditions).exe
for unit tests,@dir/runtest
alias for CRAM tests)Implementation
Core Architecture
Tests.T
(unit tests) andCram_stanza.T
(CRAM tests)collect_test_stanzas
) from transformation (describe_stanza
) for clarity and maintainability_build/default/<dir>/<name>.exe
@<source_dir>/runtest
(alias target)Dyn
serialization for consistency with other describe subcommands--format=csexp
)enabled
status (the boolean value indicates whether the test would run)Technical Details
Cram_stanza
viadune_rules
module for type matchingContext_name.Map
,Import.Main
)Memo.O
usage to avoid operator conflicts withCmdliner.Term
Testing
Added comprehensive blackbox test coverage in
test/blackbox-tests/test-cases/describe/describe-tests.t
:Phase 1: Unit Tests (Tests.T)
enabled_if
)Phase 2: CRAM Tests (Cram_stanza.T)
Fixes
Closes #12030