Re: Git Test Coverage Report (Friday, Nov 2)
Michał Górny writes: > I've just did a little research and came to the following results: Wonderful. > 1. modifying the 'C. O. Mitter' key would require changes to 4 tests, > > 2. modifying the 'Eris Discordia' key would require changes to 2 tests >(both in 7510). > > Do you think 2. would be an acceptable option? Yeah, that sounds like the best way to go. Thanks for digging.
Re: Git Test Coverage Report (Friday, Nov 2)
On Sat, 2018-11-03 at 19:03 +0900, Junio C Hamano wrote: > Michał Górny writes: > > > As for how involved... we'd just have to use a key that has split > > signing subkey. Would it be fine to add the subkey to the existing key? > > It would imply updating keyids/fingerprints everywhere. > > Yes, that "everywhere" is exactly what I meant by "how involved", > and your suggestion answers "very much involved". > > If we can easily add _another_ key with a subkey that is not the > primary one we use for other tests, without touching the existing > key and the existing tests that use it (including the one I touched > below--- we'd want to see a sig with a key that is not split is > shown with the same %GF and %GP), while adding a handful of new > tests that create signed objects under the new & split key and > view them with %GF and %GP, then the end result would be that we > managed to add a new test case where %GF/%GP are different without > making very much involved changes. I guess that was what I was > getting at. > I've just did a little research and came to the following results: 1. modifying the 'C. O. Mitter' key would require changes to 4 tests, 2. modifying the 'Eris Discordia' key would require changes to 2 tests (both in 7510). Do you think 2. would be an acceptable option? I think changing 2 tests would be preferable to proliferating a third key for one test case. Also, given that both failing tests are specifically format string tests, one of them would serve additional purpose of testing %GP!=%GF. -- Best regards, Michał Górny signature.asc Description: This is a digitally signed message part
Re: Git Test Coverage Report (Friday, Nov 2)
On Fri, Nov 02, 2018 at 10:16:48PM -0400, Derrick Stolee wrote: > Here is the coverage report for today. Some builds were timing out, so I > removed the tests with number 9000 or more from the build [1]. Hopefully > this is a temporary measure. I think it's the Azure CI patch series, see: https://public-inbox.org/git/20181017232952.gt19...@szeder.dev/ https://public-inbox.org/git/20181021112053.gc30...@szeder.dev/
Re: Git Test Coverage Report (Friday, Nov 2)
Michał Górny writes: > As for how involved... we'd just have to use a key that has split > signing subkey. Would it be fine to add the subkey to the existing key? > It would imply updating keyids/fingerprints everywhere. Yes, that "everywhere" is exactly what I meant by "how involved", and your suggestion answers "very much involved". If we can easily add _another_ key with a subkey that is not the primary one we use for other tests, without touching the existing key and the existing tests that use it (including the one I touched below--- we'd want to see a sig with a key that is not split is shown with the same %GF and %GP), while adding a handful of new tests that create signed objects under the new & split key and view them with %GF and %GP, then the end result would be that we managed to add a new test case where %GF/%GP are different without making very much involved changes. I guess that was what I was getting at. Thanks. > >> >> t/t7510-signed-commit.sh | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh >> index 19ccae2869..9ecafedcc4 100755 >> --- a/t/t7510-signed-commit.sh >> +++ b/t/t7510-signed-commit.sh >> @@ -176,8 +176,9 @@ test_expect_success GPG 'show good signature with custom >> format' ' >> 13B6F51ECDDE430D >> C O Mitter >> 73D758744BE721698EC54E8713B6F51ECDDE430D >> +73D758744BE721698EC54E8713B6F51ECDDE430D >> EOF >> -git log -1 --format="%G?%n%GK%n%GS%n%GF" sixth-signed >actual && >> +git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" sixth-signed >actual && >> test_cmp expect actual >> ' >>
Re: Git Test Coverage Report (Friday, Nov 2)
On Sat, 2018-11-03 at 12:38 +0900, Junio C Hamano wrote:
> Derrick Stolee writes:
>
> > Uncovered code in 'next' not in 'master'
> >
> >
> > pretty.c
> > 4de9394dcb 1264) if (c->signature_check.primary_key_fingerprint)
> > 4de9394dcb 1265) strbuf_addstr(sb,
> > c->signature_check.primary_key_fingerprint);
> > 4de9394dcb 1266) break;
>
> Perhaps a patch along this line can be appended to the
> mg/gpg-fingerprint topic that ends at 4de9394d ("gpg-interface.c:
> obtain primary key fingerprint as well", 2018-10-22) to cover this
> entry in the report.
>
> I do not know how involved it would be to set up a new test case
> that demonstrates a case where %GF and %GP are different, but if it
> is very involved perhaps it is not worth adding such a case.
Well, I didn't add a test for %GP primarily because we didn't have a key
with different primary and subkey fingerprints.
As for how involved... we'd just have to use a key that has split
signing subkey. Would it be fine to add the subkey to the existing key?
It would imply updating keyids/fingerprints everywhere.
>
> t/t7510-signed-commit.sh | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
> index 19ccae2869..9ecafedcc4 100755
> --- a/t/t7510-signed-commit.sh
> +++ b/t/t7510-signed-commit.sh
> @@ -176,8 +176,9 @@ test_expect_success GPG 'show good signature with custom
> format' '
> 13B6F51ECDDE430D
> C O Mitter
> 73D758744BE721698EC54E8713B6F51ECDDE430D
> + 73D758744BE721698EC54E8713B6F51ECDDE430D
> EOF
> - git log -1 --format="%G?%n%GK%n%GS%n%GF" sixth-signed >actual &&
> + git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" sixth-signed >actual &&
> test_cmp expect actual
> '
>
--
Best regards,
Michał Górny
signature.asc
Description: This is a digitally signed message part
Re: Git Test Coverage Report (Friday, Nov 2)
Derrick Stolee writes:
> Uncovered code in 'next' not in 'master'
>
>
> pretty.c
> 4de9394dcb 1264) if (c->signature_check.primary_key_fingerprint)
> 4de9394dcb 1265) strbuf_addstr(sb,
> c->signature_check.primary_key_fingerprint);
> 4de9394dcb 1266) break;
Perhaps a patch along this line can be appended to the
mg/gpg-fingerprint topic that ends at 4de9394d ("gpg-interface.c:
obtain primary key fingerprint as well", 2018-10-22) to cover this
entry in the report.
I do not know how involved it would be to set up a new test case
that demonstrates a case where %GF and %GP are different, but if it
is very involved perhaps it is not worth adding such a case.
t/t7510-signed-commit.sh | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 19ccae2869..9ecafedcc4 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -176,8 +176,9 @@ test_expect_success GPG 'show good signature with custom
format' '
13B6F51ECDDE430D
C O Mitter
73D758744BE721698EC54E8713B6F51ECDDE430D
+ 73D758744BE721698EC54E8713B6F51ECDDE430D
EOF
- git log -1 --format="%G?%n%GK%n%GS%n%GF" sixth-signed >actual &&
+ git log -1 --format="%G?%n%GK%n%GS%n%GF%n%GP" sixth-signed >actual &&
test_cmp expect actual
'
Git Test Coverage Report (Friday, Nov 2)
Here is the coverage report for today. Some builds were timing out, so I
removed the tests with number 9000 or more from the build [1]. Hopefully
this is a temporary measure.
Thanks,
-Stolee
[1]
https://dev.azure.com/git/git/_build/results?buildId=250&_a=summary=logs
---
pu: 44234e885f450a57812806cefe0d4aa3f43f2800
jch: 35594a3ecc09908facb9790ba3817ad08d2af8e1
next: 96ac06677ac950c97d331f0ef4892125027eff39
master: d582ea202b626dcc6c3b01e1e11a296d9badd730
master@{1}: 4ede3d42dfb57f9a41ac96a1f216c62eb7566cc2
Uncovered code in 'pu' not in 'jch'
--
builtin/blame.c
44234e885f builtin/blame.c 200)
repo_unuse_commit_buffer(the_repository, commit, message);
74e8221b52 builtin/blame.c 924) blame_date_width = sizeof("Thu Oct 19
16:00");
74e8221b52 builtin/blame.c 925) break;
builtin/describe.c
44234e885f builtin/describe.c 257) repo_parse_commit(the_repository, p);
builtin/fsck.c
255a564e58 builtin/fsck.c 622) fprintf_ln(stderr, _("Checking %s link"),
head_ref_name);
255a564e58 builtin/fsck.c 627) return error(_("invalid %s"), head_ref_name);
builtin/grep.c
76e9bdc437 builtin/grep.c 429) grep_read_unlock();
builtin/pack-objects.c
44234e885f builtin/pack-objects.c 2832) if
(!repo_has_object_file(the_repository, >oid) &&
is_promisor_object(>oid))
builtin/reflog.c
c9ef0d95eb builtin/reflog.c 585) all_worktrees = 0;
c9ef0d95eb builtin/reflog.c 621) continue;
date.c
74e8221b52 113) die("Timestamp too large for this system: %"PRItime, time);
74e8221b52 216) if (tm->tm_mon == human_tm->tm_mon) {
74e8221b52 217) if (tm->tm_mday > human_tm->tm_mday) {
74e8221b52 219) } else if (tm->tm_mday == human_tm->tm_mday) {
74e8221b52 220) hide.date = hide.wday = 1;
74e8221b52 221) } else if (tm->tm_mday + 5 > human_tm->tm_mday) {
74e8221b52 223) hide.date = 1;
74e8221b52 231) gettimeofday(, NULL);
74e8221b52 232) show_date_relative(time, tz, , buf);
74e8221b52 233) return;
74e8221b52 246) hide.seconds = 1;
74e8221b52 247) hide.tz |= !hide.date;
74e8221b52 248) hide.wday = hide.time = !hide.year;
74e8221b52 262) strbuf_rtrim(buf);
74e8221b52 287) gettimeofday(, NULL);
74e8221b52 290) human_tz = local_time_tzoffset(now.tv_sec, _tm);
74e8221b52 886) static int auto_date_style(void)
74e8221b52 888) return (isatty(1) || pager_in_use()) ? DATE_HUMAN :
DATE_NORMAL;
74e8221b52 909) return DATE_HUMAN;
74e8221b52 911) return auto_date_style();
fast-import.c
44234e885f 2935) buf = repo_read_object_file(the_repository, oid, ,
);
44234e885f 3041) buf = repo_read_object_file(the_repository, oid, ,
fsck.c
44234e885f 858) repo_unuse_commit_buffer(the_repository, commit, buffer);
44234e885f 878) repo_read_object_file(the_repository,
44234e885f 879) >object.oid, , );
http-push.c
44234e885f 1635) if (!repo_has_object_file(the_repository, _oid))
44234e885f 1642) if (!repo_has_object_file(the_repository,
_ref->old_oid))
merge-recursive.c
4cdc48e412 1585) return -1;
4cdc48e412 1588) return -1;
4cdc48e412 1594) return -1;
4cdc48e412 1596) if (update_file(o, 1, b_oid, b_mode, collide_path))
4cdc48e412 1597) return -1;
4cdc48e412 1664) return -1;
4cdc48e412 1667) return -1;
4cdc48e412 1670) return -1;
b58ae691c0 1703) return -1;
387361a6a7 1738) return -1;
387361a6a7 1786) return -1;
387361a6a7 1795) new_path = unique_path(o, a->path, ci->branch1);
387361a6a7 1796) output(o, 1, _("Refusing to lose untracked file"
387361a6a7 1802) return -1;
387361a6a7 1805) return -1;
387361a6a7 1815) return -1;
387361a6a7 1831) return -1;
387361a6a7 1834) return -1;
negotiator/default.c
44234e885f 71) if (repo_parse_commit(the_repository, commit))
refs.c
3a3b9d8cde 657) return 0;
refs/files-backend.c
remote.c
879b6a9e6f 1140) return error(_("dst ref %s receives from more than one
src."),
revision.c
44234e885f 726) if (repo_parse_commit(the_repository, p) < 0)
sequencer.c
44234e885f 1624) repo_unuse_commit_buffer(the_repository, head_commit,
44234e885f 3868) repo_unuse_commit_buffer(the_repository,
sha1-array.c
bba406749a 91) oidcpy([dst], [src]);
submodule-config.c
bcbc780d14 740) return CONFIG_INVALID_KEY;
45f5ef3d77 755) warning(_("Could not update .gitmodules entry %s"), key);
submodule.c
b303ef65e7 524) the_repository->submodule_prefix :
e2419f7e30 1378) strbuf_release();
7454fe5cb6 1501) struct get_next_submodule_task *task = task_cb;
7454fe5cb6 1505) get_next_submodule_task_release(task);
7454fe5cb6 1532) return 0;
7454fe5cb6 1536) goto out;
7454fe5cb6 1551) return 0;
tree.c
44234e885f 108) if (repo_parse_commit(the_repository, commit))
worktree.c
3a3b9d8cde 495) return -1;
3a3b9d8cde 508) return -1;
3a3b9d8cde 517) return -1;
ab3e1f78ae 537) break;
wrapper.c
5efde212fc 70) die("Out of memory, malloc failed (tried to allocate %"
PRIuMAX " bytes)",
5efde212fc 73) error("Out of memory, malloc failed (tried to allocate
%" PRIuMAX " bytes)",
Commits introducing uncovered code:
Ævar Arnfjörð Bjarmason 879b6a9e6: i18n: remote.c: mark
