#1160: fix undefined method 'zero?' in dimensions-of-terrain judge

[Yegorov]

Test fail, because this zerocracy/fbe#341 PR need merge before
Closes #1160

Summary by CodeRabbit

• Bug Fixes
  • Enhanced repository size validation across metrics calculations to safely handle missing and zero values
  • Improved robustness when processing repositories with incomplete data to prevent potential errors
  • Extended test coverage to validate correct handling of repositories with missing size information

[coderabbitai]

Walkthrough

The PR adds nil-aware checks for repository size values in dimension-of-terrain judges to prevent NoMethodError when calling .zero? on nil sizes. Test coverage is updated to include a repository with nil size.

Changes

Cohort / File(s)	Summary
Nil-safe repository size checks in judges judges/dimensions-of-terrain/total_commits.rb, judges/dimensions-of-terrain/total_contributors.rb, judges/dimensions-of-terrain/total_files.rb	Added nil checks before .zero? calls on repository size to prevent NoMethodError when size is nil or zero
Test coverage expansion test/judges/test-dimensions-of-terrain.rb	Updated test repository list to include yegor256/nil-size-repo alongside existing repositories to verify nil-size handling

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Homogeneous changes across all files (consistent pattern of adding nil-safe guards)
• Straightforward logic additions with no control flow restructuring
• Minimal file scope and low complexity density
• Clear bug fix addressing the linked NoMethodError issue

Suggested reviewers

• yegor256

Poem

A rabbit hops through repos with care,
Checking sizes both nil and spare,
No more errors when nil appears,
Guard clauses calm our code-review fears! 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "#1160: fix undefined method 'zero?' in dimensions-of-terrain judge" accurately and concisely describes the main change. The summary shows that the PR modifies three judge files (total_commits.rb, total_contributors.rb, and total_files.rb) to add nil-aware checks before calling the zero? method, and updates the test file to include nil-size scenarios. The title directly reflects this fix and is specific enough for a developer to understand the primary change without being overly detailed.
Linked Issues Check	✅ Passed	The linked issue #1160 describes a NoMethodError occurring when calling zero? on nil values in total_commits.rb. The PR changes directly address this requirement by modifying the guard conditions in total_commits.rb, total_contributors.rb, and total_files.rb to check for nil before calling zero?, preventing the undefined method error. Additionally, the test file is updated to include a nil-size-repo test case, providing test coverage for the nil scenario that triggered the original error.
Out of Scope Changes Check	✅ Passed	All code changes in the PR are directly related to fixing the NoMethodError reported in issue #1160. The three judge files (total_commits.rb, total_contributors.rb, total_files.rb) are modified exclusively to add nil-checking guards, and the test file is updated to include nil-size test cases. No unrelated changes, refactoring, or scope creep are present in the PR.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

judges/dimensions-of-terrain/total_files.rb (1)

11-11: Update comment to reflect nil-size handling.

The comment mentions "ignores empty repositories" but now the code also skips repositories with nil size. Consider updating the comment for accuracy.

Apply this diff to update the comment:

-# items (files, not directories), and ignores empty repositories.
+# items (files, not directories), and ignores empty and nil-size repositories.

judges/dimensions-of-terrain/total_contributors.rb (1)

12-12: Update comment to reflect nil-size handling.

The comment states "Empty repositories (size zero) are skipped" but now the code also skips repositories with nil size. Update the comment for accuracy.

Apply this diff:

-# each contributor is counted only once even if they contribute to multiple repositories.
-# Empty repositories (size zero) are skipped.
+# each contributor is counted only once even if they contribute to multiple repositories.
+# Empty and nil-size repositories are skipped.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9f6f4c and 0c60c8f.

📒 Files selected for processing (4)

• judges/dimensions-of-terrain/total_commits.rb (1 hunks)
• judges/dimensions-of-terrain/total_contributors.rb (1 hunks)
• judges/dimensions-of-terrain/total_files.rb (1 hunks)
• test/judges/test-dimensions-of-terrain.rb (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

test/judges/test-dimensions-of-terrain.rb (1)

test/test__helper.rb (1)

• load_it (46-56)

judges/dimensions-of-terrain/total_commits.rb (2)

judges/quantity-of-deliverables/total_commits_pushed.rb (1)

• total_commits_pushed (15-29)

test/judges/test-quantity-of-deliverables.rb (1)

• test_counts_commits (20-88)

🪛 GitHub Actions: rake

test/judges/test-dimensions-of-terrain.rb

[error] 288-288: TestDimensionsOfTerrain#test_total_commits failed. Expected: 1484, Actual: 2968.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: make

🔇 Additional comments (3)

judges/dimensions-of-terrain/total_files.rb (1)

23-23: LGTM - nil check prevents NoMethodError.

The addition of the nil check correctly prevents the NoMethodError that occurs when calling .zero? on a nil value, which was the root cause of issue #1160.

judges/dimensions-of-terrain/total_contributors.rb (1)

23-24: LGTM - nil check prevents NoMethodError.

The nil-aware guard correctly prevents calling .zero? on nil values. The refactoring to fetch repository info first is reasonable, though it differs slightly from the pattern in total_files.rb (which uses inline access). Both approaches work correctly.

judges/dimensions-of-terrain/total_commits.rb (1)

20-21: LGTM - nil check prevents NoMethodError.

The nil-aware guard correctly addresses the bug reported in issue #1160 by preventing .zero? from being called on nil values.

[kreinba]

The nil guard is added in three judges but the test suite only exercises the new branch in one of them, and the make and rake checks on the head commit are red. Two inline comments below.

[kreinba]

The repositories list now includes yegor256/nil-size-repo for test_total_commits, but test_total_contributors (line 500) and test_total_files (line 417) still pass only foo/foo,yegor256/empty-repo, so the new nil? branch added to total_contributors.rb and total_files.rb is never executed by the suite. Add the same nil-size repository to those two tests so every changed file has a regression test.

[kreinba]

The single Fbe.octo.repository(repo) call was split into a local variable to read :size twice — fine — but the new branch is not covered by a test. Without a stub returning size: nil for a repo passed to test_total_contributors, this guard is dead code from the suite's point of view, and the rake check on the current head is failing.

[yegor256]

This pull request has merge conflicts. Please rebase it on the default branch to resolve them so it can be merged.