Skip to content

Conversation

@billf
Copy link
Contributor

@billf billf commented May 22, 2025

Improved VersionMap tests and documentation

This PR enhances the VersionMap implementation by:

  1. Improving documentation for the insert method to clarify its behavior with alternates
  2. Splitting the monolithic test into focused test cases:
    • Basic operations (insertion, duplicate handling)
    • Alternate lookups (exact vs alternate version matching)
    • Latest version operations
    • Removal functionality
  3. Adding comprehensive tests for the version_alternate function to verify behavior with:
    • Pre-release versions
    • Major versions > 0
    • Minor versions > 0 (when major is 0)
    • Patch versions (when major and minor are 0)

@billf billf mentioned this pull request May 22, 2025
Copy link
Contributor Author

billf commented May 22, 2025

This comment was marked as outdated.

@billf billf force-pushed the billf/semver/test branch from 51dbaa4 to 17b55a3 Compare May 23, 2025 22:08
@billf billf force-pushed the billf/semver/base branch from 03f3e94 to 05a8696 Compare May 23, 2025 22:08
@billf billf changed the base branch from billf/semver/base to graphite-base/3 May 23, 2025 22:18
@billf billf force-pushed the billf/semver/test branch from 17b55a3 to cb10ffc Compare May 23, 2025 22:19
@billf billf force-pushed the graphite-base/3 branch from 05a8696 to 48abe88 Compare May 23, 2025 22:19
@billf billf changed the base branch from graphite-base/3 to master May 23, 2025 22:19
@billf billf force-pushed the billf/semver/test branch 4 times, most recently from 9b3f645 to 2505633 Compare May 29, 2025 21:15
@billf billf self-assigned this May 29, 2025
@billf billf requested a review from a team May 29, 2025 21:17
@billf billf force-pushed the billf/semver/test branch 5 times, most recently from 03ade8b to 33f88fe Compare June 3, 2025 18:38
@billf billf marked this pull request as ready for review June 3, 2025 19:10
@billf billf force-pushed the billf/semver/test branch from 33f88fe to a33b9ad Compare June 3, 2025 19:13
@billf billf requested review from Copilot and dylanplecki June 3, 2025 22:02
Copy link

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

This PR refines the VersionMap module by enhancing its documentation and substantially expanding test coverage.

  • Clarifies the behavior of insert with alternate versions
  • Splits the previous monolithic test into focused test cases (basic ops, alternates, latest, removal)
  • Adds thorough tests for version_alternate across pre-release, major, minor, and patch scenarios
Comments suppressed due to low confidence (2)

src/semver.rs:285

  • [nitpick] The variable names v1 and v2 are inconsistent with the earlier version0..version3 naming. Consider renaming them to version1 and version2 for clarity and consistency.
let v1 = Version::new(1, 0, 0);

src/semver.rs:296

  • Consider adding an explicit test for version_alternate(&Version::new(0, 0, 0)) to verify and document the behavior when both major and minor are zero.
fn test_version_alternate_function() {

Comment on lines +269 to +271
map.insert(Version::new(1, 0, 0), "v1.0.0");
map.insert(Version::new(2, 0, 0), "v2.0.0");
map.insert(Version::new(0, 1, 0), "v0.1.0");
Copy link

Copilot AI Jun 3, 2025

Choose a reason for hiding this comment

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

[nitpick] Tests mix the infallible insert API with the fallible try_insert. For consistency and clearer error handling, consider using try_insert in all test cases.

Suggested change
map.insert(Version::new(1, 0, 0), "v1.0.0");
map.insert(Version::new(2, 0, 0), "v2.0.0");
map.insert(Version::new(0, 1, 0), "v0.1.0");
assert!(map.try_insert(Version::new(1, 0, 0), "v1.0.0").is_ok());
assert!(map.try_insert(Version::new(2, 0, 0), "v2.0.0").is_ok());
assert!(map.try_insert(Version::new(0, 1, 0), "v0.1.0").is_ok());

Copilot uses AI. Check for mistakes.
@billf billf force-pushed the billf/semver/test branch from a33b9ad to 84e2f75 Compare June 3, 2025 23:43
@billf billf merged commit 3809588 into master Jun 3, 2025
3 checks passed
@github-actions
Copy link

github-actions bot commented Jun 3, 2025

Code Coverage Report

Package Base Coverage New Coverage Difference
graph.rs 🟢 84.58% 🟢 84.58% ⚪ 0%
path.rs 🟢 100% 🟢 100% ⚪ 0%
semver.rs 🟢 97.7% 🟢 97.7% ⚪ 0%
trampoline.rs 🟠 70.83% 🟠 70.83% ⚪ 0%
Overall Coverage 🟢 86.06% 🟢 86.06% ⚪ 0%

Minimum allowed coverage is 0%, this run produced 86.06%

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.

2 participants