Split up core/test/iter.rs into multiple files

[danii]

This PR removes the // ignore-tidy-filelength at the top of iter.rs by splitting it into several sub files. I have split the file per test, based on what I felt the test was really trying to test.
Addresses issue #60302.

• associated_util.rs - For testing the module level functions. (Maybe should be renamed?)
• collection.rs - For testing methods that use the values of all the items in an iterator, and creates one value from them, whether it be a Vec of all the values, or a number that represents the sum.
• mod.rs - For utility structs used in all tests, and other tests I didn't know where to place.
• range.rs - For testing ranges.
• transformation.rs - For testing methods that transform all the values in an iterator.
• util.rs - For testing methods that provide utility in more specific use cases.

"Programatically"

You may have noticed I "Programatically" split up the file. This is how I managed to do that. 😛 Hope that's fine.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[Mark-Simulacrum]

r? @cuviper who has done a lot with iterators so is likely better able to gauge if this splitting is the right one or even helpful :)

[cuviper]

I'm fine with the idea of splitting this up, but the current subdivisions don't make much sense to me. The rough principal I would use is how easy someone could guess where to find related test cases. Like, suppose I just fixed something in Chain, then is it obvious where I would find tests focused on chain? (Hopefully they're together!)

(At the end of the day, this will all be very subjective, but it's also just tests -- not an API we have to commit to.)

• associated_util.rs - For testing the module level functions. (Maybe should be renamed?)

This group seems fine, but I'd probably just call it functions.rs.

• collection.rs - For testing methods that use the values of all the items in an iterator, and creates one value from them, whether it be a Vec of all the values, or a number that represents the sum.

There's one test_collect that's on the nose, but the rest of these don't make sense to me. There are tests of folds, sums, products, min/max -- I would not have thought to look in collection.rs for any of these.

• mod.rs - For utility structs used in all tests, and other tests I didn't know where to place.

That's fine for mod.rs to be the default catch-all for tests, but I wouldn't bother with the mod other nesting. As for the utility structs, I think they only belong here if they would be shared by multiple different test modules, otherwise keep them close to their tests so it's clear what those are doing.

• range.rs - For testing ranges.

This one is good, they're all range iterators.

• transformation.rs - For testing methods that transform all the values in an iterator.

This is ambiguous to me -- I wouldn't guess what to find in there, and it looks like quite a mix.

• util.rs - For testing methods that provide utility in more specific use cases.

This too, I find unintuitive.

I guess the point is that I don't really care what a test is doing, since they are usually invented scenarios that aren't the point. What matters is to group them by what they are testing. If I modify Chain or Zip, I want to find all the tests centered around that particular iterator. Stuff like min/max and maybe eq/lt/etc. can go together, but I would make it obvious like ordering.rs.

I think it's fine if these end up even smaller, rather than trying to define very broad groups. If it's too small (subjective), just leave it at the top level.

[cuviper]

You could just mirror the layout of core/src/iter/, so the subjective organization is already decided for you, assuming you can figure out the focus of each particular test.

[bors]

☔ The latest upstream changes (presumably #76912) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

[danii]

I've taken your suggestion to mirror the layout of core/src/iter to heart, and have decided to resort everything in fns.txt to programatically split it again.
Do you mind reviewing my fns.txt in case there's any other big changes you want me to make?
Format of fns.txt is simple, <resulting file> fn <function name in iter.rs> for each line.
Yes, those file names do include ::, but I'll move them manually into the right folders and rename them afterwards.

I have tried to mirror it as close as I can, but it did get fuzzy. I decided to put tests relating to a trait or an adapter in the module corresponding to it. I also decided to add new modules for some things if they ended up in the modules core::iter::traits and core::iter::adapters (excluding submodules). If you'd like, I can remove those extra submodules.

I feel this is definitely a lot better from my first attempt, because they aren't split into broad groups anymore, and it did feel like that was a bit of an issue. :)

[cuviper]

Do you mind reviewing my fns.txt in case there's any other big changes you want me to make?

This seems to be gone now, 404.

[danii]

Oh oops, I recently changed my name...
https://gist.github.com/danii/a58b3bcafa9faf1c3e4b01ad7495baf4
Surprised that doesn't temporarily redirect like repository links.

[cuviper]

@danii That breakdown looks good to me!

[crlf0710]

@danii Ping from triage, this seems need a rebase to continue. Would you mind doing so? Thanks!

[danii]

@rustbot label -S-waiting-on-author +S-waiting-on-review

After a season, this should be ready to rebase, I hope.

[bors]

☔ The latest upstream changes (presumably #81018) made this pull request unmergeable. Please resolve the merge conflicts.

[bors]

☔ The latest upstream changes (presumably #81152) made this pull request unmergeable. Please resolve the merge conflicts.

[cuviper]

It's difficult to be sure that nothing is lost, but I'm concerned that the net line count is reduced:

33 files changed, 3574 insertions(+), 3681 deletions(-)

If anything, I'd expect it to increase for the repetition of use statements in each new file. At least some of the loss will be simple whitespace between functions though, which I would like to see fixed.

[rust-log-analyzer]

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

configure: rust.channel         := nightly
configure: rust.debug-assertions := True
configure: llvm.assertions      := True
configure: dist.missing-tools   := True
configure: build.configure-args := ['--enable-sccache', '--disable-manage-submodu ...
configure: writing `config.toml` in current directory
configure: 
configure: run `python /checkout/x.py --help`
configure: 
---
    Checking std v0.0.0 (/checkout/library/std)
error[E0583]: file not found for module `step`
  --> library/core/tests/iter/adapters/mod.rs:18:1
   |
18 | mod step;
   |
   |
   = help: to create the module `step`, create file "library/core/tests/iter/adapters/step.rs"
error: aborting due to previous error

For more information about this error, try `rustc --explain E0583`.
error: could not compile `core`
error: could not compile `core`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed
command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "i686-pc-windows-gnu" "-Zbinary-dep-depinfo" "-j" "16" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace compiler-builtins-c" "--manifest-path" "/checkout/library/test/Cargo.toml" "--all-targets" "-p" "test" "-p" "std" "-p" "unwind" "-p" "alloc" "-p" "proc_macro" "-p" "panic_abort" "-p" "core" "-p" "panic_unwind" "-p" "term" "--message-format" "json-render-diagnostics"
failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check --target=i686-pc-windows-gnu --host=i686-pc-windows-gnu --all-targets
Build completed unsuccessfully in 0:00:40

[cuviper]

Looks great, thanks! The added documentation is very welcome too.

[cuviper]

@bors r+

[bors]

📌 Commit 0c78500 has been approved by cuviper

[danii]

Ay, nice! I know I sat on this pull request for too long, but now I'm feeling great. Thank you.

[bors]

⌛ Testing commit 0c78500 with merge fe0fa59...

[bors]

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing fe0fa59 to master...