hive-forge: test the two invariants dependency.rs documents but never checked
dep_meta's comment records a measured 404: leaving owner/repo as None made every same-repo dependency fail, because forgejo resolves the dependency's repo from the request BODY rather than the URL. Nothing stopped a later tidy-up from simplifying them back to None. index() promises "error instead of wrapping". A wrap would not fail loudly -- it would hand the forge a negative index it looks up as some other issue.
This commit is contained in:
parent
e0c2f1aeaa
commit
74b48c5afa
2 changed files with 41 additions and 1 deletions
|
|
@ -464,7 +464,7 @@ fn check_status(resp: Response, op: &str) -> Result<Response> {
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::{owner_repo_from_git_url, split_repo};
|
use super::{index, owner_repo_from_git_url, split_repo};
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn owner_repo_from_git_url_strips_scheme_and_creds() {
|
fn owner_repo_from_git_url_strips_scheme_and_creds() {
|
||||||
|
|
@ -525,4 +525,19 @@ mod tests {
|
||||||
assert!(split_repo("a/b/c").is_err());
|
assert!(split_repo("a/b/c").is_err());
|
||||||
assert!(split_repo("").is_err());
|
assert!(split_repo("").is_err());
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The doc comment promises "error instead of wrapping". A wrap
|
||||||
|
/// would not fail loudly — it would produce a *negative* index the
|
||||||
|
/// forge then looks up as some other issue, so the failure mode is a
|
||||||
|
/// wrong answer rather than an error.
|
||||||
|
#[test]
|
||||||
|
fn index_errors_out_of_range_rather_than_wrapping() {
|
||||||
|
assert_eq!(index(0).unwrap(), 0);
|
||||||
|
assert_eq!(index(3950).unwrap(), 3950);
|
||||||
|
// The largest number that still fits, and the first that does not.
|
||||||
|
let max = u64::try_from(i64::MAX).expect("i64::MAX is non-negative");
|
||||||
|
assert_eq!(index(max).unwrap(), i64::MAX);
|
||||||
|
assert!(index(max + 1).is_err());
|
||||||
|
assert!(index(u64::MAX).is_err());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -186,3 +186,28 @@ fn dep_meta(owner: &str, repo: &str, dep: u64) -> Result<IssueMeta> {
|
||||||
repo: Some(repo.to_owned()),
|
repo: Some(repo.to_owned()),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
|
||||||
|
/// The comment above `dep_meta` records a measured failure: leaving
|
||||||
|
/// `owner`/`repo` as `None` made every same-repo dependency 404,
|
||||||
|
/// because forgejo resolves the dependency's repo from the *body*
|
||||||
|
/// rather than falling back to the request URL. Nothing but a test
|
||||||
|
/// stops a future tidy-up from "simplifying" them back to `None`.
|
||||||
|
#[test]
|
||||||
|
fn dep_meta_populates_owner_and_repo_rather_than_leaving_them_none() {
|
||||||
|
let m = dep_meta("hyperhive", "hyperhive", 3950).expect("in range");
|
||||||
|
assert_eq!(m.owner.as_deref(), Some("hyperhive"));
|
||||||
|
assert_eq!(m.repo.as_deref(), Some("hyperhive"));
|
||||||
|
assert_eq!(m.index, Some(3950));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Out-of-range must be an error, not a wrapped negative index that
|
||||||
|
/// the forge would then look up as some other issue.
|
||||||
|
#[test]
|
||||||
|
fn dep_meta_refuses_a_number_no_forge_could_have() {
|
||||||
|
assert!(dep_meta("o", "r", u64::MAX).is_err());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue