On 12/26/2019 12:19 PM, Derrick Stolee wrote:
> Here is the test-coverage report for v2.25.0-rc0 against v2.24.1.
> add-interactive.c
> add-patch.c
These two files had a lot of changes, but they are related to
the "builtin add -p" feature, which is still not complete. Hence,
tests do not cover this logic.
> builtin/bundle.c
> 73c3253d 143) static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix) {
> 73c3253d 145) int bundle_fd = -1;
> 73c3253d 147) struct option options[] = {
> 73c3253d 152) argc = parse_options_cmd_bundle(argc, argv, prefix,
> 73c3253d 156) memset(&header, 0, sizeof(header));
> 73c3253d 157) if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)
> 73c3253d 158) return 1;
> 73c3253d 159) if (!startup_info->have_repository)
> 73c3253d 161) return !!unbundle(the_repository, &header, bundle_fd, 0) ||
> 73c3253d 162) list_bundle_refs(&header, argc, argv);
> 73c3253d 187) else if (!strcmp(argv[0], "unbundle"))
> 73c3253d 188) result = cmd_bundle_unbundle(argc, argv, prefix);
It is unfortunate that we never had tests for "git bundle unbundle",
but this code is just a refactor of the previously-untested code.
> dir.c
> 96cc8ab5 663) pl->use_cone_patterns = 0;
> 96cc8ab5 665) goto clear_hashmaps;
> 190a65f9 676) strihash(translated->pattern) :
> 96cc8ab5 689) hashmap_add(&pl->parent_hashmap, &translated->ent);
> 96cc8ab5 690) hashmap_remove(&pl->recursive_hashmap, &translated->ent, &data);
> 96cc8ab5 691) free(data);
> 96cc8ab5 692) return;
> 96cc8ab5 698) goto clear_hashmaps;
> 190a65f9 707) strihash(translated->pattern) :
> 96cc8ab5 716) hashmap_remove(&pl->parent_hashmap, &translated->ent, &data);
> 96cc8ab5 717) free(data);
> 96cc8ab5 718) free(translated);
> 96cc8ab5 1280) return MATCHED;
Many of these lines are related to verifying cone-mode patterns
in the sparse-checkout file. I plan to send a series that makes
these checks more robust, and that will include more tests that
should improve coverage here. I'll try to get that out before
the release candidate period to ensure these uncovered lines
do not contain a bug that needs fixing.
> progress.c
> 44a4693b 308) return start_progress_delay(title, total, get_default_delay(), 1);
This is the one line in start_delayed_sparse_progress(), which
has no callers. Shall we delete?
> revision.c
> 0aa0c2b2 3202) static void reset_topo_walk(struct rev_info *revs)
> 0aa0c2b2 3204) struct topo_walk_info *info = revs->topo_walk_info;
> 0aa0c2b2 3206) clear_prio_queue(&info->explore_queue);
> 0aa0c2b2 3207) clear_prio_queue(&info->indegree_queue);
> 0aa0c2b2 3208) clear_prio_queue(&info->topo_queue);
> 0aa0c2b2 3209) clear_indegree_slab(&info->indegree);
> 0aa0c2b2 3210) clear_author_date_slab(&info->author_date);
> 0aa0c2b2 3212) FREE_AND_NULL(revs->topo_walk_info);
> 0aa0c2b2 3213) }
> 0aa0c2b2 3220) reset_topo_walk(revs);
We call reset_topo_walk(revs) on line 3220 only if we run
init_topo_walk() a second time on the same revs (and revs->topo_walk_info
is non-null). 0aa0c2b2 allowed this, but it appears nothing needs it right
now. Looks safe, anyway.
> strbuf.c
> b38dd9e7 1129) int strbuf_edit_interactively(struct strbuf *buffer, const char *path,
(more add-interactive stuff here)
> t/helper/test-run-command.c
(these changes are Windows-specific)
Those are the things that caught my eye. All new behavior
is looking pretty well tested. Thanks, everyone!
Thanks,
-Stolee
