Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

index: fix change id resolution test to not depend on deterministic order #3045

Merged
merged 1 commit into from
Feb 14, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
index: fix change id resolution test to not depend on deterministic o…
…rder

Since IdIndex sorts the entries by using .sort_unstable_by_key(), the order of
the same-key elements is undefined. Perhaps, it's stable for short arrays, and
the test passes because of that.
yuja committed Feb 14, 2024
commit 71ac1807c15cf97b11f07160e1c7b1729ec73405
2 changes: 2 additions & 0 deletions lib/src/index.rs
Original file line number Diff line number Diff line change
@@ -127,6 +127,8 @@ pub trait MutableIndex {

pub trait ChangeIdIndex: Send + Sync {
/// Resolve an unambiguous change ID prefix to the commit IDs in the index.
///
/// The order of the returned commit IDs is unspecified.
fn resolve_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>>;

/// This function returns the shortest length of a prefix of `key` that
34 changes: 21 additions & 13 deletions lib/tests/test_index.rs
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::collections::HashSet;
use std::fs;
use std::sync::Arc;

@@ -28,6 +29,7 @@ use jj_lib::object_id::{HexPrefix, ObjectId as _, PrefixResolution};
use jj_lib::op_store::{RefTarget, RemoteRef};
use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
use jj_lib::settings::UserSettings;
use maplit::hashset;
use testutils::test_backend::TestBackend;
use testutils::{
commit_transactions, create_random_commit, load_repo_at_head, write_random_commit,
@@ -780,36 +782,39 @@ fn test_change_id_index() {
assert_eq!(prefix_len(&commit_3), 6);
assert_eq!(prefix_len(&commit_4), 1);
assert_eq!(prefix_len(&commit_5), 1);
let resolve_prefix =
|prefix: &str| change_id_index.resolve_prefix(&HexPrefix::new(prefix).unwrap());
let resolve_prefix = |prefix: &str| {
change_id_index
.resolve_prefix(&HexPrefix::new(prefix).unwrap())
.map(HashSet::from_iter)
};
// Ambiguous matches
assert_eq!(resolve_prefix("a"), PrefixResolution::AmbiguousMatch);
assert_eq!(resolve_prefix("aaaaa"), PrefixResolution::AmbiguousMatch);
// Exactly the necessary length
assert_eq!(
resolve_prefix("0"),
PrefixResolution::SingleMatch(vec![root_commit.id().clone()])
PrefixResolution::SingleMatch(hashset! {root_commit.id().clone()})
);
assert_eq!(
resolve_prefix("aaaaaa"),
PrefixResolution::SingleMatch(vec![commit_3.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_3.id().clone()})
);
assert_eq!(
resolve_prefix("aaaaab"),
PrefixResolution::SingleMatch(vec![commit_2.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_2.id().clone()})
);
assert_eq!(
resolve_prefix("ab"),
PrefixResolution::SingleMatch(vec![commit_1.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_1.id().clone()})
);
assert_eq!(
resolve_prefix("b"),
PrefixResolution::SingleMatch(vec![commit_5.id().clone(), commit_4.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_4.id().clone(), commit_5.id().clone()})
);
// Longer than necessary
assert_eq!(
resolve_prefix("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"),
PrefixResolution::SingleMatch(vec![commit_3.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_3.id().clone()})
);
// No match
assert_eq!(resolve_prefix("ba"), PrefixResolution::NoMatch);
@@ -822,19 +827,22 @@ fn test_change_id_index() {
assert_eq!(prefix_len(&commit_1), 2);
assert_eq!(prefix_len(&commit_2), 2);
assert_eq!(prefix_len(&commit_3), 6);
let resolve_prefix =
|prefix: &str| change_id_index.resolve_prefix(&HexPrefix::new(prefix).unwrap());
let resolve_prefix = |prefix: &str| {
change_id_index
.resolve_prefix(&HexPrefix::new(prefix).unwrap())
.map(HashSet::from_iter)
};
assert_eq!(
resolve_prefix("0"),
PrefixResolution::SingleMatch(vec![root_commit.id().clone()])
PrefixResolution::SingleMatch(hashset! {root_commit.id().clone()})
);
assert_eq!(
resolve_prefix("aa"),
PrefixResolution::SingleMatch(vec![commit_2.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_2.id().clone()})
);
assert_eq!(
resolve_prefix("ab"),
PrefixResolution::SingleMatch(vec![commit_1.id().clone()])
PrefixResolution::SingleMatch(hashset! {commit_1.id().clone()})
);
assert_eq!(resolve_prefix("a"), PrefixResolution::AmbiguousMatch);
assert_eq!(resolve_prefix("b"), PrefixResolution::NoMatch);