Skip to content

Move remaining core specs to use fewer shared examples#1369

Open
Earlopain wants to merge 13 commits into
ruby:masterfrom
Earlopain:fewer-shared-specs-2
Open

Move remaining core specs to use fewer shared examples#1369
Earlopain wants to merge 13 commits into
ruby:masterfrom
Earlopain:fewer-shared-specs-2

Conversation

@Earlopain
Copy link
Copy Markdown
Contributor

It's quite chonky but mostly more of the same for #1364. This converts the entirety of core specs.

Before:
3805 files, 35698 examples, 273269 expectations, 0 failure, 0 errors, 0 tagged

After:
3805 files, 34586 examples, 256944 expectations, 0 failure, 0 errors, 0 tagged

1112 fewer specs.

@Earlopain Earlopain force-pushed the fewer-shared-specs-2 branch 2 times, most recently from f311e61 to f51cf1d Compare June 1, 2026 17:50
@eregon eregon requested a review from Copilot June 1, 2026 19:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@Earlopain
Copy link
Copy Markdown
Contributor Author

Let me split this up in two PRs then

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 150 out of 150 changed files in this pull request and generated 4 comments.

Comment thread core/enumerable/find_spec.rb
Comment thread core/matchdata/size_spec.rb Outdated
Comment thread core/file/fnmatch_spec.rb Outdated
Comment thread core/matchdata/equal_value_spec.rb Outdated
Comment thread core/enumerable/map_spec.rb Outdated

describe "Enumerable#map" do
it_behaves_like :enumerable_collect, :map
it "is an alias of Enumerable#collect" do
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the other way around, i.e. map/select/include? are way more used than collect/find_all/member? so I think they should have the specs and the less used aliases should just check they are aliases of the most-used methods.
The other aliases in Enumerable look good in terms of the most-used having the specs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something that could be fixed later, we could merge both PRs and I could review all usages of is an alias of to see which don't match that rule.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to put this in the blame, I'll take care of it in this PR. It's what rdoc considers the alias but I don't mind choosing the more popular version instead.

I'll do that tomorrow or so, already pretty late for me now.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 150 out of 150 changed files in this pull request and generated 4 comments.

Comment thread core/kernel/to_enum_spec.rb
Comment thread core/kernel/to_enum_spec.rb
Comment thread core/env/select_spec.rb Outdated
Comment thread core/env/select_spec.rb Outdated
@Earlopain Earlopain force-pushed the fewer-shared-specs-2 branch from 83885f1 to a62129f Compare June 2, 2026 11:20
@Earlopain Earlopain requested a review from Copilot June 2, 2026 11:20
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 150 out of 150 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (5)

core/io/tty_spec.rb:1

  • This spec can be flaky across CI/container environments because /dev/tty may be missing or inaccessible (e.g., Errno::ENOENT, Errno::EACCES, Errno::ENODEV), not just Errno::ENXIO. Consider broadening the rescued errors (or guarding by checking existence/permissions) so the spec reliably skips when TTY access is not available.
    core/file/to_path_spec.rb:1
  • This platform_is/guard block defines before/after hooks but contains no examples that use @dir. Either add the missing TMPFILE-related examples within this scope or remove the unused hooks to avoid dead setup code.
    core/float/fdiv_spec.rb:1
  • The second example’s description duplicates the first one verbatim, which makes failures harder to interpret. Rename the second description to reflect the distinct intent (e.g., division by a very large Integer).
    core/kernel/dup_spec.rb:1
  • The spec that verified #dup calls #initialize_copy was removed, which drops coverage for an important and observable part of Ruby’s object-copying semantics. Consider re-adding an explicit example asserting initialize_copy is invoked (or adding equivalent coverage elsewhere in the same spec suite) so regressions in dup hooks are caught.
    core/kernel/clone_spec.rb:1
  • Similarly to dup_spec, removing the initialize_copy expectation for #clone reduces coverage for clone’s copy hook behavior. Re-introduce an example that asserts initialize_copy is called during clone to keep coverage of this user-visible contract.

Comment thread core/enumerable/reduce_spec.rb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants