These test files for [ranges.access] are all pretty similar, and hiding them in one-file-per-directory doesn't accomplish anything (except perhaps hiding the fact that we don't have very many tests for cbegin and cend). Pull them all up to the same level, so we can see that they're all present and accounted for.
Details
- Reviewers
Mordante philnik var-const jloser ldionne • Quuxplusone - Group Reviewers
Restricted Project - Commits
- rG6842f52a0bbf: [libc++] [test] Flatten the directory structure a bit. NFC.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I don't necessarily mind moving them, but after this changes searching for a path containing range.prim turns up empty.
I would prefer names that contain the section name, for example
libcxx/test/std/ranges/range.access/range.access.begin.pass.cpp
libcxx/test/std/ranges/range.access/range.prim.data.pass.cpp
As discussed on Discord I'm oke with this, but let's discuss it with @ldionne after the holidays.
Can someone summarize the discussion? Previously the test paths were mirroring section names in the standard. I'd like to understand what the proposed naming scheme is.
The new test paths mirror the section names in the standard: everything that is specified under http://eel.is/c++draft/range.access is now located under libcxx/test/*/ranges/range.access/.
$ ls ../libcxx/test/*/ranges/range.access/ ../libcxx/test/libcxx/ranges/range.access/: begin.incomplete_type.sh.cpp end.incomplete_type.pass.cpp ../libcxx/test/std/ranges/range.access/: begin.pass.cpp data.pass.cpp empty.pass.cpp end.pass.cpp size.pass.cpp ssize.pass.cpp
The old test paths were kind of wonky, because they included subdirectory components that didn't mirror the standard's stablenames. E.g. data, size, and ssize were previously tested under the subdirectory libcxx/test/std/ranges/range.access/range.prim/, but there is no https://eel.is/c++draft/range.prim in the standard.
@Mordante suggested that it might be better to name the test files after the corresponding stablenames, instead of the corresponding library identifiers:
begin.pass.cpp => range.access.begin.pass.cpp end.pass.cpp => range.access.end.pass.cpp data.pass.cpp => range.prim.data.pass.cpp empty.pass.cpp => range.prim.empty.pass.cpp size.pass.cpp => range.prim.size.pass.cpp ssize.pass.cpp => range.prim.ssize.pass.cpp
I'm only like 80% opposed to that idea. :) The current names are shorter/easier to decode (e.g. empty is tested in empty.pass.cpp, not something with the word empty buried in the middle of the name). The short names are also slightly better at hiding the contradiction that cbegin is tested in the same file as begin: it seems pretty defensible that begin.pass.cpp tests cbegin too, whereas if we call it range.access.begin.pass.cpp then we have to explain the absence of range.access.cbegin.pass.cpp. (This applies to end/cend, rbegin/crbegin, rend/crend, and data/cdata. It should probably also apply to size/ssize, but we haven't gotten around to rewriting that pair of test files into a single file yet. I suspect a lot of test coverage is missing for ssize.)
Okay, in that case my affirmation above was wrong. Thanks for clarifying.
@Mordante suggested that it might be better to name the test files after the corresponding stablenames, instead of the corresponding library identifiers:
begin.pass.cpp => range.access.begin.pass.cpp end.pass.cpp => range.access.end.pass.cpp data.pass.cpp => range.prim.data.pass.cpp empty.pass.cpp => range.prim.empty.pass.cpp size.pass.cpp => range.prim.size.pass.cpp ssize.pass.cpp => range.prim.ssize.pass.cpp
I'm not a huge fan of this either, for the same reasons you stated. It would be more consistent if we did this throughout, though, but it would be a pretty big change (a simple change but it would touch a lot of files). Today, we basically use section names except for the leaf name where we use "whatever makes sense", or at least that's how I would codify what I think we're doing.
Thanks for your input.
I basically wanted that style since it keeps the omitted section names, including the odd "prim". But I've no objection against the shorter names.