Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@xxxxxxxxxx>
>
> The creationToken heuristic uses a different mechanism for downloading
> bundles from the "standard" approach. Specifically: it uses a concrete
> order based on the creationToken values and attempts to download as few
> bundles as possible. It also modifies local config to store a value for
> future fetches to avoid downloading bundles, if possible.
>
> However, if any of the individual bundles has a failed download, then
> the logic for the ordering comes into question. It is important to avoid
> infinite loops, assigning invalid creation token values in config, but
> also to be opportunistic as possible when downloading as many bundles as
> seem appropriate.
>
> These tests were used to inform the implementation of
> fetch_bundles_by_token() in bundle-uri.c, but are being added
> independently here to allow focusing on faulty downloads. There may be
> more cases that could be added that result in modifications to
> fetch_bundles_by_token() as interesting data shapes reveal themselves in
> real scenarios.
>
The expanded testing is great, thanks for adding it!
> + # Case 2: middle bundle does not exist, only two bundles can unbundle
> + cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
> + [bundle]
> + version = 1
> + mode = all
> + heuristic = creationToken
> +
> + [bundle "bundle-1"]
> + uri = bundle-1.bundle
> + creationToken = 1
> +
> + [bundle "bundle-2"]
> + uri = fake.bundle
> + creationToken = 2
> +
> + [bundle "bundle-3"]
> + uri = bundle-3.bundle
> + creationToken = 3
> +
> + [bundle "bundle-4"]
> + uri = bundle-4.bundle
> + creationToken = 4
> + EOF
> +
> + GIT_TRACE2_EVENT="$(pwd)/trace-clone-2.txt" \
> + git clone --single-branch --branch=base \
> + --bundle-uri="$HTTPD_URL/bundle-list" \
> + "$HTTPD_URL/smart/fetch.git" download-2 &&
> +
> + # Bundle failure does not set these configs.
> + test_must_fail git -C download-2 config fetch.bundleuri &&
> + test_must_fail git -C download-2 config fetch.bundlecreationtoken &&
> +
> + cat >expect <<-EOF &&
> + $HTTPD_URL/bundle-list
> + $HTTPD_URL/bundle-4.bundle
> + $HTTPD_URL/bundle-3.bundle
> + $HTTPD_URL/fake.bundle
> + $HTTPD_URL/bundle-1.bundle
> + EOF
> + test_remote_https_urls <trace-clone-2.txt >actual &&
> + test_cmp expect actual &&
> +
> + # Only base bundle unbundled.
> + git -C download-2 for-each-ref --format="%(refname)" "refs/bundles/*" >refs &&
> + cat >expect <<-EOF &&
> + refs/bundles/base
> + refs/bundles/right
> + EOF
> + test_cmp expect refs &&
Maybe I'm misreading, but I don't think the comment ("Only base bundle
unbundled") lines up with the expected bundle refs (both bundle-1
('refs/bundles/base') and bundle-3 ('refs/bundles/right') seem to be
unbundled).
> +
> + # Case 3: top bundle does not exist, rest unbundle fine.
> + cat >"$HTTPD_DOCUMENT_ROOT_PATH/bundle-list" <<-EOF &&
> + [bundle]
> + version = 1
> + mode = all
> + heuristic = creationToken
> +
> + [bundle "bundle-1"]
> + uri = bundle-1.bundle
> + creationToken = 1
> +
> + [bundle "bundle-2"]
> + uri = bundle-2.bundle
> + creationToken = 2
> +
> + [bundle "bundle-3"]
> + uri = bundle-3.bundle
> + creationToken = 3
> +
> + [bundle "bundle-4"]
> + uri = fake.bundle
> + creationToken = 4
> + EOF
> +
> + GIT_TRACE2_EVENT="$(pwd)/trace-clone-3.txt" \
> + git clone --single-branch --branch=base \
> + --bundle-uri="$HTTPD_URL/bundle-list" \
> + "$HTTPD_URL/smart/fetch.git" download-3 &&
> +
> + # As long as we have continguous successful downloads,
> + # we _do_ set these configs.
> + test_cmp_config -C download-3 "$HTTPD_URL/bundle-list" fetch.bundleuri &&
> + test_cmp_config -C download-3 3 fetch.bundlecreationtoken &&
> +
> + cat >expect <<-EOF &&
> + $HTTPD_URL/bundle-list
> + $HTTPD_URL/fake.bundle
> + $HTTPD_URL/bundle-3.bundle
> + $HTTPD_URL/bundle-2.bundle
> + $HTTPD_URL/bundle-1.bundle
> + EOF
> + test_remote_https_urls <trace-clone-3.txt >actual &&
> + test_cmp expect actual &&
> +
> + # All bundles failed to unbundle
> + git -C download-3 for-each-ref --format="%(refname)" "refs/bundles/*" >refs &&
> + cat >expect <<-EOF &&
> + refs/bundles/base
> + refs/bundles/left
> + refs/bundles/right
> + EOF
> + test_cmp expect refs
Similar issue with the comment here - it says that all bundles *failed* to
unbundle, but the test case description ("Case 3: top bundle does not exist,
rest unbundle fine.") and the result show bundle-1, bundle-2, and bundle-3
all unbundling successfully.
> +'
> +
> +# Expand the bundle list to include other interesting shapes, specifically
> +# interesting for use when fetching from a previous state.
> +#
> +# ---------------- bundle-7
> +# 7
> +# _/|\_
> +# ---/--|--\------ bundle-6
> +# 5 | 6
> +# --|---|---|----- bundle-4
> +# | 4 |
> +# | / \ /
> +# --|-|---|/------ bundle-3 (the client will be caught up to this point.)
> +# \ | 3
> +# ---\|---|------- bundle-2
> +# 2 |
> +# ----|---|------- bundle-1
> +# \ /
> +# 1
> +# |
> +# (previous commits)
...
> + # Case 1: all bundles exist: successful unbundling of all bundles
...
> + # Case 2: middle bundle does not exist, only bundle-4 can unbundle
...
> + # Case 3: top bundle does not exist, rest unbundle fine.
The rest of these cases look okay and, at a high-level, it's helpful to have
these additional tests covering a different topology.
