{"thread":{"id":"51198","subject":"Git Test Coverage Report (Thursday, May 30th)","startedAt":"2019-05-30T12:52:45Z","lastAt":"2019-06-10T15:15:16Z","messageCount":11,"participants":["Derrick Stolee","Johannes Schindelin","Michael Platings","Barret Rhoden"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"376439","messageId":"2fb43bd3-71a7-fd92-e9b8-43e4eeed34cd@gmail.com","threadId":"51198","inReplyTo":null,"subject":"Git Test Coverage Report (Thursday, May 30th)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-05-30T12:52:41Z","receivedAt":"2019-05-30T12:52:45Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Here is today's test coverage report. You can view it in HTML [1] or\ndownload the plain-text version [2], also pasted below.\n\nThanks,\n-Stolee\n\n[1] https://derrickstolee.github.io/git-test-coverage/reports/2019-05-30.htm\n[2] https://derrickstolee.github.io/git-test-coverage/reports/2019-05-30.txt\n\n---\n\npu\t1feae2a559816e6df62e994d4ffdc226dee6631a\njch\t1605b8c0b5622510a96ef6a358adbd142cf8222e\nnext\t5d7573a151b918062087822a2f0d7661dc4bd707\nmaster\taa25c82427ae70aebf3b8f970f2afd54e9a2a8c6\nmaster@{1}\tab15ad1a3b4b04a29415aef8c9afa2f64fc194a2\n\n\nUncovered code in 'pu' not in 'jch'\n--------------------------------------------------------\n\nblame.c\n170072f9 846)     (result[i] >= most_certain_line_a ||\n170072f9 847)      second_best_result[i] >= most_certain_line_a)) {\n170072f9 848) certainties[i] = CERTAINTY_NOT_CALCULATED;\n170072f9 951) max_search_distance_b = 0;\n1fc73384 998) return;\n8934ac8c 1190)     ent->ignored == next->ignored &&\n8934ac8c 1191)     ent->unblamable == next->unblamable) {\n43885768 1607) continue;\nae3f36de 2425) continue;\n\nbuiltin/blame.c\n\nbuiltin/cat-file.c\n\nbuiltin/fetch-pack.c\n\nbuiltin/pack-objects.c\n\nbuiltin/rebase.c\nd559f502 759) ret = error(_(\"could not remove '%s'\"),\nd559f502 1675) error(_(\"could not remove '%s'\"),\n\nbuiltin/remote.c\nf39a9c65 1551) die(_(\"--save-to-push cannot be used with other options\"));\nf39a9c65 1575) die(_(\"--save-to-push can only be used when only one url is defined\"));\n\nfetch-pack.c\n\nhttp-fetch.c\n\nhttp.c\nee334603 2302) target ? hash_to_hex(target->hash) : base_url,\n\noidset.c\n\npromisor-remote.c\n0ba08c05 25) die(_(\"Remote with no URL\"));\n54248706 61) warning(_(\"promisor remote name cannot begin with '/': %s\"),\n54248706 63) return NULL;\n7bdf0926 93) previous->next = r->next;\n7b6e1b04 108) return git_config_string(&core_partial_clone_filter_default,\nb21a55f3 139) return 0;\ndcc8b4e9 202) static int remove_fetched_oids(struct object_id **oids, int oid_nr, int to_free)\ndcc8b4e9 204) int i, missing_nr = 0;\ndcc8b4e9 205) int *missing = xcalloc(oid_nr, sizeof(*missing));\ndcc8b4e9 206) struct object_id *old_oids = *oids;\ndcc8b4e9 208) int old_fetch_if_missing = fetch_if_missing;\ndcc8b4e9 210) fetch_if_missing = 0;\ndcc8b4e9 212) for (i = 0; i < oid_nr; i++)\ndcc8b4e9 213) if (oid_object_info_extended(the_repository, &old_oids[i], NULL, 0)) {\ndcc8b4e9 214) missing[i] = 1;\ndcc8b4e9 215) missing_nr++;\ndcc8b4e9 218) fetch_if_missing = old_fetch_if_missing;\ndcc8b4e9 220) if (missing_nr) {\ndcc8b4e9 221) int j = 0;\ndcc8b4e9 222) new_oids = xcalloc(missing_nr, sizeof(*new_oids));\ndcc8b4e9 223) for (i = 0; i < oid_nr; i++)\ndcc8b4e9 224) if (missing[i])\ndcc8b4e9 225) oidcpy(&new_oids[j++], &old_oids[i]);\ndcc8b4e9 226) *oids = new_oids;\ndcc8b4e9 227) if (to_free)\ndcc8b4e9 228) free(old_oids);\ndcc8b4e9 231) free(missing);\ndcc8b4e9 233) return missing_nr;\ndcc8b4e9 248) if (missing_nr == 1)\ndcc8b4e9 249) continue;\ndcc8b4e9 250) missing_nr = remove_fetched_oids(&missing_oids, missing_nr, to_free);\ndcc8b4e9 251) if (missing_nr) {\ndcc8b4e9 252) to_free = 1;\ndcc8b4e9 253) continue;\ndcc8b4e9 261) free(missing_oids);\n\nprotocol.c\n\nref-filter.c\n2582083f 93) keydata_aka_refname ? keydata_aka_refname : k->wt->head_ref);\n\nremote-curl.c\n\nsequencer.c\n37e9ee5c 293) ret = -1;\n37e9ee5c 311) ret = error(_(\"could not remove '%s'\"), buf.buf);\n\nupload-pack.c\na8d662e3 130) return readsz;\n820a5361 149) BUG(\"packfile_uris requires sideband-all\");\na8d662e3 355) send_client_data(1, output_state.buffer, output_state.used);\n820a5361 1386) string_list_clear(&data->uri_protocols, 0);\n\nwrapper.c\n\nCommits introducting uncovered code:\nBarret Rhoden\t1fc73384 blame: optionally track line fingerprints during fill_blame_origin()\nBarret Rhoden\t8934ac8c blame: add config options for the output of ignored or unblamable lines\nBarret Rhoden\t43885768 blame: use the fingerprint heuristic to match ignored lines\nBarret Rhoden\tae3f36de blame: add the ability to ignore commits and their changes\nChristian Couder\t7b6e1b04 Move core_partial_clone_filter_default to promisor-remote.c\nChristian Couder\tdcc8b4e9 promisor-remote: implement promisor_remote_get_direct()\nChristian Couder\tb21a55f3 promisor-remote: parse remote.*.partialclonefilter\nChristian Couder\t7bdf0926 promisor-remote: use repository_format_partial_clone\nChristian Couder\t54248706 Add initial support for many promisor remotes\nChristian Couder\t0ba08c05 Remove fetch-object.{c,h} in favor of promisor-remote.{c,h}\nDenton Liu\tf39a9c65 remote: add --save-to-push option to git remote set-url\nJonathan Tan\ta8d662e3 upload-pack: refactor reading of pack-objects out\nJonathan Tan\t820a5361 upload-pack: send part of packfile response as uri\nJunio C Hamano\tee334603 Merge branch 'jt/fetch-cdn-offload' into pu\nMichael Platings\t170072f9 blame: add a fingerprint heuristic to match ignored lines\nNickolai Belakovski\t2582083f ref-filter: add worktreepath atom\nPhillip Wood\td559f502 rebase --abort/--quit: cleanup refs/rewritten\nPhillip Wood\t37e9ee5c sequencer: return errors from sequencer_remove_state()\n\n\nUncovered code in 'jch' not in 'next'\n--------------------------------------------------------\n\nbuiltin/branch.c\n1fde99cf 833) die(_(\"The -a, and -r, options to 'git branch' do not take a branch name.\\n\"\n\nbuiltin/checkout.c\n\nbuiltin/commit.c\n\nbuiltin/gc.c\nefeb229e 691) return 1;\n\nbuiltin/rebase.c\n526c03b5 1262) goto done;\n10572de1 1278) goto done;\n\ncommit-graph.c\nd83160e8 906) error(_(\"error opening index for %s\"), packname.buf);\nd83160e8 907) return 1;\n63a8be62 946) continue;\n93ba1867 969) display_progress(ctx->progress, ctx->approx_nr_objects);\n8520d7fc 1039) error(_(\"unable to create leading directories of %s\"),\n8520d7fc 1041) return errno;\nefeb229e 1158) error(_(\"the commit graph format cannot write %d commits\"), count_distinct);\nefeb229e 1159) res = 1;\nefeb229e 1160) goto cleanup;\nefeb229e 1169) error(_(\"too many commits to write graph\"));\nefeb229e 1170) res = 1;\nefeb229e 1171) goto cleanup;\n\nread-cache.c\nee70c128 1723) if (advice_unknown_index_extension) {\nee70c128 1724) warning(_(\"ignoring optional %.4s index extension\"), ext);\nee70c128 1725) advise(_(\"This is likely due to the file having been written by a newer\\n\"\n\nCommits introducting uncovered code:\nDenton Liu\t526c03b5 rebase: refactor can_fast_forward into goto tower\nDenton Liu\t10572de1 rebase: fast-forward --onto in more cases\nDerrick Stolee\tefeb229e commit-graph: return with errors during write\nDerrick Stolee\td83160e8 commit-graph: extract fill_oids_from_packs()\nDerrick Stolee\t63a8be62 commit-graph: extract fill_oids_from_commit_hex()\nDerrick Stolee\t93ba1867 commit-graph: extract fill_oids_from_all_packs()\nDerrick Stolee\t8520d7fc commit-graph: extract write_commit_graph_file()\nJonathan Nieder\tee70c128 index: offer advice for unknown index extensions\nPhilip Oakley\t1fde99cf doc branch: provide examples for listing remote tracking branches\n\n\nUncovered code in 'next' not in 'master'\n--------------------------------------------------------\n\nbuiltin/am.c\n97387c8b 1662) die(\"unable to read from stdin; aborting\");\n6e7baf24 2336) die(_(\"interactive mode requires patches on the command line\"));\n\nbuiltin/bisect--helper.c\n7877ac3d 574) retval = error(_(\"invalid ref: '%s'\"), start_head.buf);\n7877ac3d 575) goto finish;\n\nbuiltin/fast-export.c\ne80001f8 81) static int parse_opt_reencode_mode(const struct option *opt,\ne80001f8 84) if (unset) {\ne80001f8 85) reencode_mode = REENCODE_ABORT;\ne80001f8 86) return 0;\ne80001f8 89) switch (git_parse_maybe_bool(arg)) {\ne80001f8 91) reencode_mode = REENCODE_NO;\ne80001f8 92) break;\ne80001f8 94) reencode_mode = REENCODE_YES;\ne80001f8 95) break;\ne80001f8 97) if (!strcasecmp(arg, \"abort\"))\ne80001f8 98) reencode_mode = REENCODE_ABORT;\ne80001f8 100) return error(\"Unknown reencoding mode: %s\", arg);\ne80001f8 103) return 0;\ne80001f8 665) switch(reencode_mode) {\ne80001f8 667) reencoded = reencode_string(message, \"UTF-8\", encoding);\ne80001f8 668) break;\ne80001f8 670) break;\ne80001f8 672) die(\"Encountered commit-specific encoding %s in commit \"\ne80001f8 674)     encoding, oid_to_hex(&commit->object.oid));\nccbfc96d 686) printf(\"encoding %s\\n\", encoding);\n\nbuiltin/index-pack.c\n8a30a1ef 1365) continue;\n\nbuiltin/log.c\n13cdf780 873) return 0;\n\nbuiltin/merge.c\nf3f8311e 1290) usage_msg_opt(_(\"--quit expects no arguments\"),\n\nbuiltin/rebase.c\n4c785c0e 1201) opts->flags &= ~REBASE_DIFFSTAT;\n\nbuiltin/worktree.c\n1de16aec 297) BUG(\"How come '%s' becomes empty after sanitization?\", sb.buf);\n\nbuiltin/write-tree.c\n76a7bc09 53) die(\"%s: prefix %s not found\", me, tree_prefix);\n\ndiff.c\n8ef05193 5217) return error(_(\"%s expects a numerical value\"), \"--unified\");\n\nfast-import.c\n3edfcc65 2612) read_next_command();\n3edfcc65 2679) strbuf_addf(&new_data,\n\ngrep.c\nde99eb0c 1784) BUG(\"grep call which could print a name requires \"\n\nparse-options-cb.c\nf7e68a08 20) return error(_(\"option `%s' expects a numerical value\"),\n\nparse-options.c\nf7e68a08 199) return error(_(\"%s expects a numerical value\"),\n\nprogress.c\n1aed1a5f 131)     cols - progress->title_len - 1 : 0;\n\nread-cache.c\n7bd9631b 2201) src_offset += load_cache_entries_threaded(istate, mmap, mmap_size, nr_threads, ieot);\n\nrefs.c\n1de16aec 111) sanitized->buf[sanitized->len-1] = '-';\n1de16aec 170) if (sanitized)\n1de16aec 171) strbuf_addch(sanitized, '-');\n1de16aec 173) return -1;\n1de16aec 178) strbuf_complete(sanitized, '/');\n1de16aec 215) BUG(\"sanitizing refname '%s' check returned error\", refname);\n\nserver-info.c\nf4f476b6 110) ret = -1;\nf4f476b6 111) goto out;\nf4f476b6 123) goto out;\nf4f476b6 125) goto out;\nf4f476b6 134) if (uic.cur_fp)\nf4f476b6 135) fclose(uic.cur_fp);\n\ntrace2/tr2_tls.c\n5fdae9d3 87) return pthread_getspecific(tr2tls_key) == tr2tls_thread_main;\n\nCommits introducting uncovered code:\nDenton Liu\t13cdf780 format-patch: teach format.notes config option\nElijah Newren\te80001f8 fast-export: do automatic reencoding of commit messages only if requested\nElijah Newren\tccbfc96d fast-export: avoid stripping encoding header if we cannot reencode\nElijah Newren\t3edfcc65 fast-import: support 'encoding' commit header\nEmily Shaffer\tde99eb0c grep: fail if call could output and name is null\nEric Wong\tf4f476b6 update-server-info: avoid needless overwrites\nJeff Hostetler\t5fdae9d3 trace2: fix tracing when NO_PTHREADS is defined\nJeff King\t7bd9631b read-cache: drop unused parameter from threaded load\nJeff King\t76a7bc09 cmd_{read,write}_tree: rename \"unused\" variable that is used\nJeff King\t97387c8b am: read interactive input from stdin\nJeff King\t6e7baf24 am: drop tty requirement for --interactive\nJohannes Schindelin\t7877ac3d bisect--helper: verify HEAD could be parsed before continuing\nJohannes Schindelin\t4c785c0e rebase: replace incorrect logical negation by correct bitwise one\nJonathan Tan\t8a30a1ef index-pack: prefetch missing REF_DELTA bases\nNguyễn Thái Ngọc Duy\tf3f8311e merge: add --quit\nNguyễn Thái Ngọc Duy\t8ef05193 diff-parseopt: restore -U (no argument) behavior\nNguyễn Thái Ngọc Duy\tf7e68a08 parse-options: check empty value in OPT_INTEGER and OPT_ABBREV\nNguyễn Thái Ngọc Duy\t1de16aec worktree add: sanitize worktree names\nSZEDER Gábor\t1aed1a5f progress: avoid empty line when breaking the progress line\n\n\nUncovered code in 'master' not in 'master@{1}'\n--------------------------------------------------------\n\nbuiltin/difftool.c\nf3a3a021 731) die(_(\"--dir-diff is incompatible with --no-index\"));\n\nhttp.c\n5c3d5a38 2329) return posn / eltsize;\n\nparse-options.c\n5c387428 282) return 0;\n5c387428 481) BUG(\"OPT_ALIAS() should not remain at this point. \"\n5c387428 661) BUG(\"An alias must have long option name\");\n5c387428 670) BUG(\"No please. Nested aliases are not supported.\");\n5c387428 685) BUG(\"could not find source option '%s' of alias '%s'\",\n\nsha1-name.c\naf96fe33 161) return;\n\nt/helper/test-read-cache.c\ndc76852d 29) die(\"%s not in index\", name);\n\nupload-pack.c\n8e712ef6 1073) precomposed_unicode = git_config_bool(var, value);\n\nCommits introducting uncovered code:\nDerrick Stolee\taf96fe33 midx: add packs to packed_git linked list\nElijah Newren\t8e712ef6 Honor core.precomposeUnicode in more places\nJohannes Schindelin\tf3a3a021 difftool --no-index: error out on --dir-diff (and don't crash)\nJohannes Schindelin\tdc76852d fsmonitor: demonstrate that it is not refreshed after discard_index()\nMike Hommey\t5c3d5a38 Make fread/fwrite-like functions in http.c more like fread/fwrite.\nNguyễn Thái Ngọc Duy\t5c387428 parse-options: don't emit \"ambiguous option\" for aliases\n\n"},{"id":"376448","messageId":"e18e4391-a574-1f4b-88c7-890ada116f51@gmail.com","threadId":"51198","inReplyTo":"2fb43bd3-71a7-fd92-e9b8-43e4eeed34cd@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-05-30T18:24:41Z","receivedAt":"2019-05-30T18:24:47Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"I decided to take a stab at a full review of the test coverage report in\norder to try and understand all of the uncovered code. The snippets I\nhighlight below include uncovered code that is not immediately obvious as\nan acceptable block to leave uncovered. (Some snippets required looking\naround at the context to know that is the case.)\n\nIn at least one case, I found a block that is actually covered in my\nlocal testing, so something is wrong with the build environment I use\nto generate this report. I'm currently investigating.\n\nOn 5/30/2019 8:52 AM, Derrick Stolee wrote:\n> blame.c\n> 170072f9 846)     (result[i] >= most_certain_line_a ||\n> 170072f9 847)      second_best_result[i] >= most_certain_line_a)) {\n> 170072f9 848) certainties[i] = CERTAINTY_NOT_CALCULATED;\n\nThis section appears in the following block:\n\n        /* More invalidating of results that may be affected by the choice of\n         * most certain line.\n         * Discard the matches for lines in B that are currently matched with a\n         * line in A such that their ordering contradicts the ordering imposed\n         * by the choice of most certain line.\n         */\n        for (i = most_certain_local_line_b - 1; i >= invalidate_min; --i) {\n                /* In this loop we discard results for lines in B that are\n                 * before most-certain-line-B but are matched with a line in A\n                 * that is after most-certain-line-A.\n                 */\n                if (certainties[i] >= 0 &&\n                    (result[i] >= most_certain_line_a ||\n                     second_best_result[i] >= most_certain_line_a)) {\n                        certainties[i] = CERTAINTY_NOT_CALCULATED;\n                }\n        }\n        for (i = most_certain_local_line_b + 1; i < invalidate_max; ++i) {\n                /* In this loop we discard results for lines in B that are\n                 * after most-certain-line-B but are matched with a line in A\n                 * that is before most-certain-line-A.\n                 */\n                if (certainties[i] >= 0 &&\n                    (result[i] <= most_certain_line_a ||\n                     second_best_result[i] <= most_certain_line_a)) {\n                        certainties[i] = CERTAINTY_NOT_CALCULATED;\n                }\n        }\n\nNote that the first for loop includes the uncovered lines. The logical operands\nare backwards of the conditions in the second for loop, which are covered. This\nseems non-trivial enough to merit a test.\n\n> 170072f9 951) max_search_distance_b = 0;\n> 1fc73384 998) return;\n> 8934ac8c 1190)     ent->ignored == next->ignored &&\n> 8934ac8c 1191)     ent->unblamable == next->unblamable) {\n\nThese lines are part of this diff:\n\n--- a/blame.c\n+++ b/blame.c\n@@ -479,7 +479,9 @@ void blame_coalesce(struct blame_scoreboard *sb)\n\n        for (ent = sb->ent; ent && (next = ent->next); ent = next) {\n                if (ent->suspect == next->suspect &&\n-                   ent->s_lno + ent->num_lines == next->s_lno) {\n+                   ent->s_lno + ent->num_lines == next->s_lno &&\n+                   ent->ignored == next->ignored &&\n+                   ent->unblamable == next->unblamable) {\n                        ent->num_lines += next->num_lines;\n                        ent->next = next->next;\n                        blame_origin_decref(next->suspect);\n\nThe fact that they are uncovered means that the && chain is short-circuited at\n\"ent->s_lno + ent->num_lines == next->s_lno\" before the new conditions can be\nchecked. So, the block inside is never covered. It includes a call to\nblame_origin_decref() and free(), so it would be good to try and exercise this region.\n\n> http.c\n> ee334603 2302) target ? hash_to_hex(target->hash) : base_url,\n\nThis line being uncovered means that 'target' is never NULL. In the code above,\nbase_url is used in all cases so this is safe enough.\n\n> promisor-remote.c\n\n> 7bdf0926 93) previous->next = r->next;\n\nThis isn't being hit because \"previous\" is always NULL in the call to\npromisor_remote_move_to_tail(), which is filled by a call to\npromisor_remote_lookup(). All of this code is rather difficult to read\n(double pointers, for loops with two iterator variables) so it is hard\nto do the mental math and guarantee that it is working.\n\nI tried playing around with adding more promisor remotes to t0410-partial-clone.sh,\nbut could not get this line to hit.\n\n> dcc8b4e9 202) static int remove_fetched_oids(struct object_id **oids, int oid_nr, int to_free)\n\nThis method isn't covered at all, so I responded directly to the patch thread.\n\n> upload-pack.c\n> a8d662e3 355) send_client_data(1, output_state.buffer, output_state.used);\n\nThis line looks like a copy-paste from a method refactor.\n\n> 820a5361 1386) string_list_clear(&data->uri_protocols, 0);\n\nThis string_list_clear() is preceded by\n\n\tif (data->uri_protocols.nr && !data->writer.use_sideband)\n\nbut earlier is populated by \n\n                if (skip_prefix(arg, \"packfile-uris \", &p)) {\n                        string_list_split(&data->uri_protocols, p, ',', -1);\n                        continue;\n                }\n\nWhy don't we simply not use string_list_split() if !data->writer.use_sideband?\n\nI would apply this diff to avoid calling string_list_split at all:\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex db74ca57bd..267b419521 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1373,7 +1373,7 @@ static void process_args(struct packet_reader *request,\n                        continue;\n                }\n\n-               if (skip_prefix(arg, \"packfile-uris \", &p)) {\n+               if (skip_prefix(arg, \"packfile-uris \", &p) && data->writer.use_sideband) {\n                        string_list_split(&data->uri_protocols, p, ',', -1);\n                        continue;\n                }\n@@ -1381,9 +1381,6 @@ static void process_args(struct packet_reader *request,\n                /* ignore unknown lines maybe? */\n                die(\"unexpected line: '%s'\", arg);\n        }\n-\n-       if (data->uri_protocols.nr && !data->writer.use_sideband)\n-               string_list_clear(&data->uri_protocols, 0);\n }\n\n static int process_haves(struct oid_array *haves, struct oid_array *common,\n\n\n> commit-graph.c\n> 93ba1867 969) display_progress(ctx->progress, ctx->approx_nr_objects);\n\nThis line seemed suspicious, but is preceded by\n\n\tif (ctx->progress_done < ctx->approx_nr_objects)\n\nso is pretty harmless to leave uncovered.\n\n> builtin/fast-export.c\n> e80001f8 81) static int parse_opt_reencode_mode(const struct option *opt,\n\nI'm always suspicious of a method that is never called by the test suite.\nThe only caller is given by this portion of the patch:\n\n+               OPT_CALLBACK(0, \"reencode\", &reencode_mode, N_(\"mode\"),\n+                           N_(\"select handling of commit messages in an alternate encoding\"),\n+                           parse_opt_reencode_mode),\n\nBut we DO have tests that cover this flag, and inserting a die() in the\nmethod triggers it on t9350-fast-export.sh. I'll investigate what went wrong\non the build [1] to cause this. I see a lot of these in the logs:\n\n\tsh: echo: I/O error\n\nSo maybe some tests did not actually run. Further, these tests failed:\n\nt3400-rebase.sh                           (Wstat: 256 Tests: 28 Failed: 2)\n  Failed tests:  20, 28\n  Non-zero exit status: 1\nt3420-rebase-autostash.sh                 (Wstat: 256 Tests: 38 Failed: 6)\n  Failed tests:  6, 13, 16, 23, 26, 33\n  Non-zero exit status: 1\nt3404-rebase-interactive.sh               (Wstat: 256 Tests: 110 Failed: 5)\n  Failed tests:  3, 9-10, 100-101\n  Non-zero exit status: 1\nt5521-pull-options.sh                     (Wstat: 256 Tests: 19 Failed: 1)\n  Failed test:  3\n  Non-zero exit status: 1\nt5551-http-fetch-smart.sh                 (Wstat: 256 Tests: 37 Failed: 1)\n  Failed test:  26\n  Non-zero exit status: 1\n\nThey don't fail locally, so perhaps we shouldn't blindly trust the coverage data\nuntil I work out why these errors occurred. (Many of the cases I called out\nabove I couldn't hit locally with a die() statement.)\n\n[1] https://dev.azure.com/git/git/_build/results?buildId=606\n\n> builtin/rebase.c\n> 4c785c0e 1201) opts->flags &= ~REBASE_DIFFSTAT;\n\nThis is the only changed line in this commit, which the commit message states was\nfound by static analysis.\n\n> fast-import.c\n> 3edfcc65 2612) read_next_command();\n> 3edfcc65 2679) strbuf_addf(&new_data,\n\nI expect this is actually covered, but wasn't reported due to the issues listed above.\n\n> progress.c\n> 1aed1a5f 131)     cols - progress->title_len - 1 : 0;\n\nThis is the only changed line in this commit.\n \n> read-cache.c\n> 7bd9631b 2201) src_offset += load_cache_entries_threaded(istate, mmap, mmap_size, nr_threads, ieot);\n\nThis was previously uncovered and this commit simply changed the method prototype.\n\n> refs.c\n> 1de16aec 111) sanitized->buf[sanitized->len-1] = '-';\n> 1de16aec 170) if (sanitized)\n> 1de16aec 171) strbuf_addch(sanitized, '-');\n> 1de16aec 173) return -1;\n> 1de16aec 178) strbuf_complete(sanitized, '/');\n\nThese are special cases of custom ref types used in \"git worktree add\" and some\nof them are covered by tests, but these seem harmless.\n\nThanks,\n-Stolee\n"},{"id":"376491","messageId":"0d7dbfe6-53df-1df8-ac94-4dfab85bbc9f@gmail.com","threadId":"51198","inReplyTo":"e18e4391-a574-1f4b-88c7-890ada116f51@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-05-31T17:51:46Z","receivedAt":"2019-05-31T17:51:52Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 5/30/2019 2:24 PM, Derrick Stolee wrote:\n> Further, these tests failed\n>\n> t3400-rebase.sh                           (Wstat: 256 Tests: 28 Failed: 2)\n>   Failed tests:  20, 28\n>   Non-zero exit status: 1\n> t3420-rebase-autostash.sh                 (Wstat: 256 Tests: 38 Failed: 6)\n>   Failed tests:  6, 13, 16, 23, 26, 33\n>   Non-zero exit status: 1\n> t3404-rebase-interactive.sh               (Wstat: 256 Tests: 110 Failed: 5)\n>   Failed tests:  3, 9-10, 100-101\n>   Non-zero exit status: 1\n> t5521-pull-options.sh                     (Wstat: 256 Tests: 19 Failed: 1)\n>   Failed test:  3\n>   Non-zero exit status: 1\n> t5551-http-fetch-smart.sh                 (Wstat: 256 Tests: 37 Failed: 1)\n>   Failed test:  26\n>   Non-zero exit status: 1\n> \n> They don't fail locally, so perhaps we shouldn't blindly trust the coverage data\n> until I work out why these errors occurred. (Many of the cases I called out\n> above I couldn't hit locally with a die() statement.)\n\nThese tests all failed during the second run that set optional GIT_TEST\nenvironment variables. Specifically, GIT_TEST_REBASE_USE_BUILTIN=false\ncaused these tests to break. We now output this message:\n\n\twarning: the rebase.useBuiltin support has been removed!\n\tSee its entry in 'git help config' for details.\n\nI'm removing that variable from the build definition.\n\nThanks,\n-Stolee\n"},{"id":"376501","messageId":"nycvar.QRO.7.76.6.1905312050330.1775@tvgsbejvaqbjf.bet","threadId":"51198","inReplyTo":"0d7dbfe6-53df-1df8-ac94-4dfab85bbc9f@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-05-31T18:59:03Z","receivedAt":"2019-05-31T18:59:23Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stolee,\n\nOn Fri, 31 May 2019, Derrick Stolee wrote:\n\n> On 5/30/2019 2:24 PM, Derrick Stolee wrote:\n> > Further, these tests failed\n> >\n> > t3400-rebase.sh                           (Wstat: 256 Tests: 28 Failed: 2)\n> >   Failed tests:  20, 28\n> >   Non-zero exit status: 1\n> > t3420-rebase-autostash.sh                 (Wstat: 256 Tests: 38 Failed: 6)\n> >   Failed tests:  6, 13, 16, 23, 26, 33\n> >   Non-zero exit status: 1\n> > t3404-rebase-interactive.sh               (Wstat: 256 Tests: 110 Failed: 5)\n> >   Failed tests:  3, 9-10, 100-101\n> >   Non-zero exit status: 1\n> > t5521-pull-options.sh                     (Wstat: 256 Tests: 19 Failed: 1)\n> >   Failed test:  3\n> >   Non-zero exit status: 1\n> > t5551-http-fetch-smart.sh                 (Wstat: 256 Tests: 37 Failed: 1)\n> >   Failed test:  26\n> >   Non-zero exit status: 1\n> >\n> > They don't fail locally, so perhaps we shouldn't blindly trust the coverage data\n> > until I work out why these errors occurred. (Many of the cases I called out\n> > above I couldn't hit locally with a die() statement.)\n>\n> These tests all failed during the second run that set optional GIT_TEST\n> environment variables. Specifically, GIT_TEST_REBASE_USE_BUILTIN=false\n> caused these tests to break. We now output this message:\n>\n> \twarning: the rebase.useBuiltin support has been removed!\n> \tSee its entry in 'git help config' for details.\n>\n> I'm removing that variable from the build definition.\n\nWould it make sense to have a file in t/ (or a script-let in ci/)\nspecifying all of the `GIT_TEST_*` variables that are currently supported\n(and that actually make sense to be set)?\n\nI saw a similar issue recently in a now-defunct Azure Pipeline that also\ntried to replicate what half of the `linux-gcc` job [*1*] does: to run the\ntest suite with those variables overriding the defaults. That Pipeline\nbroke for the exact same reason you mentioned: we now handle\n`GIT_TEST_REBASE_USE_BUILTIN` by showing that warning.\n\nAnd issues like this could easily be avoided if we had, say,\n`ci/non-standard-settings.sh` that simply set all those `GIT_TEST_*`\nvariables in the way that the `linux-gcc` job does (and of course, this\njob should then source that file instead of duplicating those\nassignments).\n\nWhat do you think?\nDscho\n\nFootnote *1*: It is a thorn in my side ever since I started work on our\nAzure Pipeline support that the `linux-gcc` job actually runs *two* jobs:\nit runs the vanilla test suite, and then it runs it again after setting\nall supported `GIT_TEST_*` variables to the non-default settings. This\nalmost doubles the running time of that job, often making it the very last\njob to finish, and it also makes it unclear whether a test failure stems\nfrom said `GIT_TEST_*` settings or not.\n\nI got so annoyed by this, in fact, that I finally broke down and opened\nhttps://github.com/gitgitgadget/git/issues/242.\n"},{"id":"376536","messageId":"CAJDYR9QAWVJxn7cf1DQbnZTKeSGJPCXcTJT9FVNEDpf1bimtGg@mail.gmail.com","threadId":"51198","inReplyTo":"e18e4391-a574-1f4b-88c7-890ada116f51@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Michael Platings","fromEmail":"michael@platin.gs","sentAt":"2019-06-01T21:22:09Z","receivedAt":"2019-06-01T21:22:22Z","isPatch":false,"sender":{"key":"michael@platin.gs","avatar":"https://avatars.githubusercontent.com/u/1112348?v=4"},"body":"Thanks very much for this Derrick. I looked into it and it turns out\nthat the missing coverage in blame.c for \"certainties[i] =\nCERTAINTY_NOT_CALCULATED\" was due to earlier code overwriting the same\nvalue in most cases, thereby defeating an optimization. I've deleted\nthat earlier code and now coverage is as expected. I posted the patch\nhere: https://public-inbox.org/git/20190601210925.15339-1-michael@platin.gs/T/#u\nI also deleted the other uncovered code that appeared in the same\npatch as it was unreachable.\n\n> On 5/30/2019 8:52 AM, Derrick Stolee wrote:\n> > blame.c\n> > 170072f9 846)     (result[i] >= most_certain_line_a ||\n> > 170072f9 847)      second_best_result[i] >= most_certain_line_a)) {\n> > 170072f9 848) certainties[i] = CERTAINTY_NOT_CALCULATED;\n>\n> This section appears in the following block:\n>\n>         /* More invalidating of results that may be affected by the choice of\n>          * most certain line.\n>          * Discard the matches for lines in B that are currently matched with a\n>          * line in A such that their ordering contradicts the ordering imposed\n>          * by the choice of most certain line.\n>          */\n>         for (i = most_certain_local_line_b - 1; i >= invalidate_min; --i) {\n>                 /* In this loop we discard results for lines in B that are\n>                  * before most-certain-line-B but are matched with a line in A\n>                  * that is after most-certain-line-A.\n>                  */\n>                 if (certainties[i] >= 0 &&\n>                     (result[i] >= most_certain_line_a ||\n>                      second_best_result[i] >= most_certain_line_a)) {\n>                         certainties[i] = CERTAINTY_NOT_CALCULATED;\n>                 }\n>         }\n>         for (i = most_certain_local_line_b + 1; i < invalidate_max; ++i) {\n>                 /* In this loop we discard results for lines in B that are\n>                  * after most-certain-line-B but are matched with a line in A\n>                  * that is before most-certain-line-A.\n>                  */\n>                 if (certainties[i] >= 0 &&\n>                     (result[i] <= most_certain_line_a ||\n>                      second_best_result[i] <= most_certain_line_a)) {\n>                         certainties[i] = CERTAINTY_NOT_CALCULATED;\n>                 }\n>         }\n>\n> Note that the first for loop includes the uncovered lines. The logical operands\n> are backwards of the conditions in the second for loop, which are covered. This\n> seems non-trivial enough to merit a test.\n"},{"id":"376594","messageId":"b4ce9cbc-71d3-4d7f-1ee2-e4e3fc23b015@google.com","threadId":"51198","inReplyTo":"e18e4391-a574-1f4b-88c7-890ada116f51@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Barret Rhoden","fromEmail":"brho@google.com","sentAt":"2019-06-03T18:11:08Z","receivedAt":"2019-06-03T18:11:12Z","isPatch":false,"sender":{"key":"brho@google.com","avatar":null},"body":"Hi -\n\nOn 5/30/19 2:24 PM, Derrick Stolee wrote:\n>> 8934ac8c 1190)     ent->ignored == next->ignored &&\n>> 8934ac8c 1191)     ent->unblamable == next->unblamable) {\n> These lines are part of this diff:\n> \n> --- a/blame.c\n> +++ b/blame.c\n> @@ -479,7 +479,9 @@ void blame_coalesce(struct blame_scoreboard *sb)\n> \n>          for (ent = sb->ent; ent && (next = ent->next); ent = next) {\n>                  if (ent->suspect == next->suspect &&\n> -                   ent->s_lno + ent->num_lines == next->s_lno) {\n> +                   ent->s_lno + ent->num_lines == next->s_lno &&\n> +                   ent->ignored == next->ignored &&\n> +                   ent->unblamable == next->unblamable) {\n>                          ent->num_lines += next->num_lines;\n>                          ent->next = next->next;\n>                          blame_origin_decref(next->suspect);\n> \n> The fact that they are uncovered means that the && chain is short-circuited at\n> \"ent->s_lno + ent->num_lines == next->s_lno\" before the new conditions can be\n> checked. So, the block inside is never covered. It includes a call to\n> blame_origin_decref() and free(), so it would be good to try and exercise this region.\n\nWhat is your setup for determining if a line is uncovered?  Are you \nrunning something like gcov for all of the tests in t/?\n\nI removed this change, and none of the other blame tests appeared to \ntrigger this code block either, independently of this change.  (I put an \nassert(0) inside the block).\n\nHowever, two of our blame-ignore tests do get past the first two checks \nin the if clause, (the suspects are equal and the s_lno chunks are \nadjacent) and we do check the ignored/unblamable conditions.\n\nSpecifically, if I undo this change and put an assert(0) in that block, \ntwo of our tests hit that code, and one of our tests fails if I don't do \nthe check for ignored/unblamable.\n\nThanks,\nBarret\n\n\n\n"},{"id":"376597","messageId":"80a23fb8-5ea8-dba3-ce7d-f6f5d4c02310@gmail.com","threadId":"51198","inReplyTo":"b4ce9cbc-71d3-4d7f-1ee2-e4e3fc23b015@google.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-06-03T18:40:42Z","receivedAt":"2019-06-03T18:40:46Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/3/2019 2:11 PM, Barret Rhoden wrote:\n> Hi -\n> \n> On 5/30/19 2:24 PM, Derrick Stolee wrote:\n>>> 8934ac8c 1190)     ent->ignored == next->ignored &&\n>>> 8934ac8c 1191)     ent->unblamable == next->unblamable) {\n>> These lines are part of this diff:\n>>\n>> --- a/blame.c\n>> +++ b/blame.c\n>> @@ -479,7 +479,9 @@ void blame_coalesce(struct blame_scoreboard *sb)\n>>\n>>          for (ent = sb->ent; ent && (next = ent->next); ent = next) {\n>>                  if (ent->suspect == next->suspect &&\n>> -                   ent->s_lno + ent->num_lines == next->s_lno) {\n>> +                   ent->s_lno + ent->num_lines == next->s_lno &&\n>> +                   ent->ignored == next->ignored &&\n>> +                   ent->unblamable == next->unblamable) {\n>>                          ent->num_lines += next->num_lines;\n>>                          ent->next = next->next;\n>>                          blame_origin_decref(next->suspect);\n>>\n>> The fact that they are uncovered means that the && chain is short-circuited at\n>> \"ent->s_lno + ent->num_lines == next->s_lno\" before the new conditions can be\n>> checked. So, the block inside is never covered. It includes a call to\n>> blame_origin_decref() and free(), so it would be good to try and exercise this region.\n> \n> What is your setup for determining if a line is uncovered?  Are you running something like gcov for all of the tests in t/?\n> \n> I removed this change, and none of the other blame tests appeared to trigger this code block either, independently of this change.  (I put an assert(0) inside the block).\n> \n> However, two of our blame-ignore tests do get past the first two checks in the if clause, (the suspects are equal and the s_lno chunks are adjacent) and we do check the ignored/unblamable conditions.\n> \n> Specifically, if I undo this change and put an assert(0) in that block, two of our tests hit that code, and one of our tests fails if I don't do the check for ignored/unblamable.\n\nThe tests use gcov while running the tests in t/. Here is the build [1].\n\nThere are some i/o errors happening in the build, which I have not\nfull diagnosed. It is entirely possible that you actually are covered,\nbut there was an error collecting the coverage statistics. The simplest\nthing to do is to insert a die() statement and re-run the tests.\n\nThanks,\n-Stolee\n\n[1] https://dev.azure.com/git/git/_build/results?buildId=615\n\n"},{"id":"376673","messageId":"9ab619bb-9deb-4e57-a3ad-9e996425b783@google.com","threadId":"51198","inReplyTo":"80a23fb8-5ea8-dba3-ce7d-f6f5d4c02310@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Barret Rhoden","fromEmail":"brho@google.com","sentAt":"2019-06-04T16:38:02Z","receivedAt":"2019-06-04T16:38:07Z","isPatch":false,"sender":{"key":"brho@google.com","avatar":null},"body":"On 6/3/19 2:40 PM, Derrick Stolee wrote:\n> On 6/3/2019 2:11 PM, Barret Rhoden wrote:\n>> Hi -\n>>\n>> On 5/30/19 2:24 PM, Derrick Stolee wrote:\n>>>> 8934ac8c 1190)     ent->ignored == next->ignored &&\n>>>> 8934ac8c 1191)     ent->unblamable == next->unblamable) {\n>>> These lines are part of this diff:\n>>>\n>>> --- a/blame.c\n>>> +++ b/blame.c\n>>> @@ -479,7 +479,9 @@ void blame_coalesce(struct blame_scoreboard *sb)\n>>>\n>>>           for (ent = sb->ent; ent && (next = ent->next); ent = next) {\n>>>                   if (ent->suspect == next->suspect &&\n>>> -                   ent->s_lno + ent->num_lines == next->s_lno) {\n>>> +                   ent->s_lno + ent->num_lines == next->s_lno &&\n>>> +                   ent->ignored == next->ignored &&\n>>> +                   ent->unblamable == next->unblamable) {\n>>>                           ent->num_lines += next->num_lines;\n>>>                           ent->next = next->next;\n>>>                           blame_origin_decref(next->suspect);\n>>>\n>>> The fact that they are uncovered means that the && chain is short-circuited at\n>>> \"ent->s_lno + ent->num_lines == next->s_lno\" before the new conditions can be\n>>> checked. So, the block inside is never covered. It includes a call to\n>>> blame_origin_decref() and free(), so it would be good to try and exercise this region.\n>>\n>> What is your setup for determining if a line is uncovered?  Are you running something like gcov for all of the tests in t/?\n>>\n>> I removed this change, and none of the other blame tests appeared to trigger this code block either, independently of this change.  (I put an assert(0) inside the block).\n>>\n>> However, two of our blame-ignore tests do get past the first two checks in the if clause, (the suspects are equal and the s_lno chunks are adjacent) and we do check the ignored/unblamable conditions.\n>>\n>> Specifically, if I undo this change and put an assert(0) in that block, two of our tests hit that code, and one of our tests fails if I don't do the check for ignored/unblamable.\n> \n> The tests use gcov while running the tests in t/. Here is the build [1].\n> \n> There are some i/o errors happening in the build, which I have not\n> full diagnosed. It is entirely possible that you actually are covered,\n> but there was an error collecting the coverage statistics. The simplest\n> thing to do is to insert a die() statement and re-run the tests.\n\nIt looks like no existing tests cover that block in blame_coalesce(), \nregardless of my commit.  That's based on putting die() in there and \nrunning make in t/.  So at the worst, my patch isn't decreasing \ncoverage.  That's a pretty low bar.  =)\n\nI'll try to come up with a test, independent of my blame-ignore work, \nthat can get in that block.\n\nThanks,\n\nBarret\n\n\n"},{"id":"376692","messageId":"09bc8c7a-bab5-7cdb-75dc-f7c45a4dbaf8@google.com","threadId":"51198","inReplyTo":"9ab619bb-9deb-4e57-a3ad-9e996425b783@google.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Barret Rhoden","fromEmail":"brho@google.com","sentAt":"2019-06-04T20:41:33Z","receivedAt":"2019-06-04T20:41:38Z","isPatch":false,"sender":{"key":"brho@google.com","avatar":null},"body":"Hi -\n\nOn 6/4/19 12:38 PM, Barret Rhoden wrote:\n> I'll try to come up with a test, independent of my blame-ignore work, \n> that can get in that block.\n\nI have a test that covers blame_coalesce(), which works both with and \nwithout my blame-ignore commit that started this thread.\n\nHowever, the only thing we are really testing is that git blame didn't \ncrash.  There is no detectable change to the output.  AFAIK, \nblame_coalesce() is a performance enhancement.\n\nIf you all are interested in that sort of test, I can put it in a patch. \n  Right now, I have this (below).\n\nThanks,\n\nBarret\n\n\ndiff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh\nindex c92a47b6d5b1..4c652b85a55b 100755\n--- a/t/t8003-blame-corner-cases.sh\n+++ b/t/t8003-blame-corner-cases.sh\n@@ -275,4 +275,32 @@ test_expect_success 'blame file with CRLF \ncore.autocrlf=true' '\n  \tgrep \"A U Thor\" actual\n  '\n\n+test_expect_success 'blame coalesce' '\n+\tcat >giraffe <<-\\EOF &&\n+\tABC\n+\tDEF\n+\tEOF\n+\tgit add giraffe &&\n+\tgit commit -m \"original file\" &&\n+\t\n+\tcat >giraffe <<-\\EOF &&\n+\tABC\n+\tXXX\n+\tDEF\n+\tEOF\n+\tgit add giraffe &&\n+\tgit commit -m \"interior XXX line\" &&\n+\t\n+\tcat >giraffe <<-\\EOF &&\n+\tABC\n+\tDEF\n+\tEOF\n+\tgit add giraffe &&\n+\tgit commit -m \"same contents as original\" &&\n+\n+\tgit blame giraffe &&\n+\n+\ttrue\n+'\n+\n  test_done\n\n"},{"id":"376702","messageId":"9018d6bb-3bc1-824d-e61e-e37bb1587640@gmail.com","threadId":"51198","inReplyTo":"09bc8c7a-bab5-7cdb-75dc-f7c45a4dbaf8@google.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-06-05T00:57:13Z","receivedAt":"2019-06-05T00:57:18Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 6/4/2019 4:41 PM, Barret Rhoden wrote:\n> Hi -\n> \n> On 6/4/19 12:38 PM, Barret Rhoden wrote:\n> However, the only thing we are really testing is that git blame didn't crash. \n\nThis would not be enough.\n\n> There is no detectable change to the output.  AFAIK, blame_coalesce() is a performance enhancement.\n\nThank you for stating that the output didn't change. I\ntested this locally, and did see that the behavior was\nidentical.\n\nI think you should just make the test be complete by\nchecking a post-condition. Please see the inserted lines\nbelow (which _should_ work, I haven't actually ran this\nin the test suite).\n\n> +test_expect_success 'blame coalesce' '\n> +    cat >giraffe <<-\\EOF &&\n> +    ABC\n> +    DEF\n> +    EOF\n> +    git add giraffe &&\n> +    git commit -m \"original file\" &&\n\noid=$(git rev-parse HEAD) &&\n\n> +   \n> +    cat >giraffe <<-\\EOF &&\n> +    ABC\n> +    XXX\n> +    DEF\n> +    EOF\n> +    git add giraffe &&\n> +    git commit -m \"interior XXX line\" &&\n> +   \n> +    cat >giraffe <<-\\EOF &&\n> +    ABC\n> +    DEF\n> +    EOF\n> +    git add giraffe &&\n> +    git commit -m \"same contents as original\" &&\n> +\n\ncat >expect <<-\\EOF &&\n^$oid 1) ABC\n^$oid 2) DEF\nEOF\ngit -c core.abbrev=40 blame -s giraffe >actual &&\ntest_cmp expect actual\n\n> +'\n> +\n>  test_done\n> \n"},{"id":"376933","messageId":"8fe53b23-1fbe-882b-f52e-9b02fe6cba42@google.com","threadId":"51198","inReplyTo":"9018d6bb-3bc1-824d-e61e-e37bb1587640@gmail.com","subject":"Re: Git Test Coverage Report (Thursday, May 30th)","fromName":"Barret Rhoden","fromEmail":"brho@google.com","sentAt":"2019-06-10T15:15:11Z","receivedAt":"2019-06-10T15:15:16Z","isPatch":false,"sender":{"key":"brho@google.com","avatar":null},"body":"On 6/4/19 8:57 PM, Derrick Stolee wrote:\n[snip]\n> I think you should just make the test be complete by\n> checking a post-condition. Please see the inserted lines\n> below (which _should_ work, I haven't actually ran this\n> in the test suite).\n\nWith a little massaging, this did the trick.  I'll roll the patch into \nmy blame-ignore series, to keep things simple.\n\nThanks,\n\nBarret\n\n"}]}