Two tests for (include_subdirs qualified) and directories with invalid module names#7620
Two tests for (include_subdirs qualified) and directories with invalid module names#7620Niols wants to merge 6 commits intoocaml:mainfrom
(include_subdirs qualified) and directories with invalid module names#7620Conversation
|
related #7605 |
40d1304 to
d7e55d8
Compare
Signed-off-by: Niols <niols@niols.fr>
Signed-off-by: Niols <niols@niols.fr>
d7e55d8 to
ede03c9
Compare
Signed-off-by: Niols <niols@niols.fr>
01babf7 to
6c8836c
Compare
|
Added a test where the directory with a malformed module name is nested in a directory with a valid name and that one is the one mentioned in |
|
@anmonteiro could you lead the review on this one? |
test/blackbox-tests/test-cases/include-qualified/data-only-dirs-malformed.t/run.t
Outdated
Show resolved
Hide resolved
8af0a31 to
e1f2f4e
Compare
| @@ -0,0 +1,3 @@ | |||
| (include_subdirs qualified) | |||
| (executable (name foo)) | |||
| (data_only_dirs baz-baz) | |||
There was a problem hiding this comment.
What is this testing? I don't think this has any effect since there are no dune rules under baz-baz. (data_only_dirs ignores dune rules in the specified directories).
There are also no modules under that directory.
There was a problem hiding this comment.
I do think that if the directories contain no Dune rules and no modules, it would make sense for Dune to simply ignore them, making the tests in this PR irrelevant. At the time of writing this PR, though, a project of mine could not compile because of a situation like this, where there were no modules and no Dune rules yet Dune failed because of a directory with invalid name. I haven't tested on newer Dune version but I have just updated my branch and we'll see what the CI thinks of this.
anmonteiro
left a comment
There was a problem hiding this comment.
I think the tests would be a bit more useful if they were testing the inclusion / exclusion of the directories in question.
Right now it's not clear whether the directories are being excluded because they're specified in data_only_dirs / dirs or because they have invalid folder names.
Sorry, I don't understand what you mean!
When I wrote these tests, Dune was failing whenever a directory with an invalid name was present in the scope. I assumed it was a feature and therefore the test was supposed to exhibit the fact that Dune should not fail when the directories with invalid names were included in a |
The CI seems to show that this is still the case, but maybe I misunderstood something? |
|
To provide some closure here: |
This PR adds two tests for the
(include_subdirs qualified)stanza, specifically when this stanza is used while a directory whose name is an invalid module name is present (baz-baz). The two tests circumvent this problem in two different ways:(dirs :standard \ baz-baz)stanza to hide this module from Dune. This works fine as of now.(data_only_dirs baz-baz)stanza to specify that this directory should not be considered as a module but as a directory of data. This fails in the current state of Dune.These two tests are meant as an illustration for #7601 (comment), which might soon lead to a feature request.