* [Possible Breakage] t1308 - Bad return value from test-tool
@ 2019-02-08 21:42 Randall S. Becker
2019-02-09 4:24 ` Jeff King
0 siblings, 1 reply; 5+ messages in thread
From: Randall S. Becker @ 2019-02-08 21:42 UTC (permalink / raw)
To: git; +Cc: 'Duy Nguyen'
t1308 has me perplexed - this is an old breakage on the NonStop platform,
that I have just gotten around to checking with the new bash version we
have. When running sub-test 23, the following was reported:
Value not found for "foo.bar"
test_expect_code: command exited with 1, we wanted 2 test-tool config
configset_get_value foo.bar a-directory
However, when I looked inside t/helper/test-config.c, every path reporting
"Value not found" has a goto exit1 not exit2. It seems, from the code, that
the test is actually incorrect and should be expecting 1 not 2, and that it
is working properly on NonStop (but the test fails as a result).
What is perplexing is that someone else should be seeing this. The
test-config.c code is about 5 years old, but the test was modified a year or
two ago. What am I missing here?
Cheers,
Randall
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Possible Breakage] t1308 - Bad return value from test-tool
2019-02-08 21:42 [Possible Breakage] t1308 - Bad return value from test-tool Randall S. Becker
@ 2019-02-09 4:24 ` Jeff King
2019-02-09 18:08 ` Randall S. Becker
0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2019-02-09 4:24 UTC (permalink / raw)
To: Randall S. Becker; +Cc: git, 'Duy Nguyen'
On Fri, Feb 08, 2019 at 04:42:19PM -0500, Randall S. Becker wrote:
> t1308 has me perplexed - this is an old breakage on the NonStop platform,
> that I have just gotten around to checking with the new bash version we
> have. When running sub-test 23, the following was reported:
>
> Value not found for "foo.bar"
> test_expect_code: command exited with 1, we wanted 2 test-tool config
> configset_get_value foo.bar a-directory
>
> However, when I looked inside t/helper/test-config.c, every path reporting
> "Value not found" has a goto exit1 not exit2. It seems, from the code, that
> the test is actually incorrect and should be expecting 1 not 2, and that it
> is working properly on NonStop (but the test fails as a result).
We're expecting it to report an error reading the directory, not "value
not found". Which would yield code 2.
It sounds like you might need to set FREAD_READS_DIRECTORIES in your
config.mak.
-Peff
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [Possible Breakage] t1308 - Bad return value from test-tool
2019-02-09 4:24 ` Jeff King
@ 2019-02-09 18:08 ` Randall S. Becker
2019-02-09 23:32 ` Jeff King
0 siblings, 1 reply; 5+ messages in thread
From: Randall S. Becker @ 2019-02-09 18:08 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git, 'Duy Nguyen'
On February 8, 2019 23:24, Jeff King wrote:
> On Fri, Feb 08, 2019 at 04:42:19PM -0500, Randall S. Becker wrote:
>
> > t1308 has me perplexed - this is an old breakage on the NonStop
> > platform, that I have just gotten around to checking with the new bash
> > version we have. When running sub-test 23, the following was reported:
> >
> > Value not found for "foo.bar"
> > test_expect_code: command exited with 1, we wanted 2 test-tool config
> > configset_get_value foo.bar a-directory
> >
> > However, when I looked inside t/helper/test-config.c, every path
> > reporting "Value not found" has a goto exit1 not exit2. It seems, from
> > the code, that the test is actually incorrect and should be expecting
> > 1 not 2, and that it is working properly on NonStop (but the test fails as a
> result).
>
> We're expecting it to report an error reading the directory, not "value not
> found". Which would yield code 2.
>
> It sounds like you might need to set FREAD_READS_DIRECTORIES in your
> config.mak.
Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes
still results in
Value not found for "foo.bar"
test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory
not ok 23 - proper error on directory "files"
#
# echo "Error (-1) reading configuration file a-directory." >expect &&
# mkdir a-directory &&
# test_expect_code 2 test-tool config configset_get_value foo.bar a-directory 2>output &&
# grep "^warning:" output &&
# grep "^Error" output >actual &&
# test_cmp expect actual
#
I don't think that helped. While fopen can open a directory, fread does not return any data in this platform. readdir or nftw/ftw are pretty much the only options. However, the code still goes down the goto exit1 path in this situation.
Perplexed,
Randall
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Possible Breakage] t1308 - Bad return value from test-tool
2019-02-09 18:08 ` Randall S. Becker
@ 2019-02-09 23:32 ` Jeff King
2019-02-10 0:15 ` Randall S. Becker
0 siblings, 1 reply; 5+ messages in thread
From: Jeff King @ 2019-02-09 23:32 UTC (permalink / raw)
To: Randall S. Becker; +Cc: git, 'Duy Nguyen'
On Sat, Feb 09, 2019 at 01:08:01PM -0500, Randall S. Becker wrote:
> > It sounds like you might need to set FREAD_READS_DIRECTORIES in your
> > config.mak.
>
> Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes
Silly question, but you did rebuild after setting that, not just re-run
the tests, right?
> still results in
>
> Value not found for "foo.bar"
> test_expect_code: command exited with 1, we wanted 2 test-tool config configset_get_value foo.bar a-directory
> not ok 23 - proper error on directory "files"
> #
> # echo "Error (-1) reading configuration file a-directory." >expect &&
> # mkdir a-directory &&
> # test_expect_code 2 test-tool config configset_get_value foo.bar a-directory 2>output &&
> # grep "^warning:" output &&
> # grep "^Error" output >actual &&
> # test_cmp expect actual
> #
>
> I don't think that helped. While fopen can open a directory, fread
> does not return any data in this platform. readdir or nftw/ftw are
> pretty much the only options. However, the code still goes down the
> goto exit1 path in this situation.
Hrm. That's the exact case FREAD_READS_DIRECTORIES is supposed to help.
It does an fstat() after fopen()ing the file to see if it points to a
directory, and if so closes it immediately and returns EISDIR.
Can you confirm via debugger (or printf statements) that we make it into
git_fopen, and what the resulting st.st_mode we see looks like?
-Peff
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [Possible Breakage] t1308 - Bad return value from test-tool
2019-02-09 23:32 ` Jeff King
@ 2019-02-10 0:15 ` Randall S. Becker
0 siblings, 0 replies; 5+ messages in thread
From: Randall S. Becker @ 2019-02-10 0:15 UTC (permalink / raw)
To: 'Jeff King'; +Cc: git, 'Duy Nguyen'
On February 9, 2019 18:33, Jeff King wrote:
> On Sat, Feb 09, 2019 at 01:08:01PM -0500, Randall S. Becker wrote:
>
> > > It sounds like you might need to set FREAD_READS_DIRECTORIES in your
> > > config.mak.
> >
> > Setting FREAD_READS_DIRECTORIES = UnfortunatelyYes
>
> Silly question, but you did rebuild after setting that, not just re-run the tests,
> right?
Yes make was run, but some doofus (me) did not run make tests first, so test-tool was out of date, post reconfigure. When just a code change is made, test-tool gets build. I was/am confused.
> > still results in
> >
> > Value not found for "foo.bar"
> > test_expect_code: command exited with 1, we wanted 2 test-tool config
> > configset_get_value foo.bar a-directory not ok 23 - proper error on
> directory "files"
> > #
> > # echo "Error (-1) reading configuration file a-directory." >expect
> &&
> > # mkdir a-directory &&
> > # test_expect_code 2 test-tool config configset_get_value foo.bar
> a-directory 2>output &&
> > # grep "^warning:" output &&
> > # grep "^Error" output >actual &&
> > # test_cmp expect actual
> > #
> >
> > I don't think that helped. While fopen can open a directory, fread
> > does not return any data in this platform. readdir or nftw/ftw are
> > pretty much the only options. However, the code still goes down the
> > goto exit1 path in this situation.
>
> Hrm. That's the exact case FREAD_READS_DIRECTORIES is supposed to help.
> It does an fstat() after fopen()ing the file to see if it points to a directory, and
> if so closes it immediately and returns EISDIR.
>
> Can you confirm via debugger (or printf statements) that we make it into
> git_fopen, and what the resulting st.st_mode we see looks like?
It did not get into git_fopen at all. Subtest 1 did, but that's no surprise.
t1308 is passes now, finally. Patch coming.
Thanks for the help,
Randall
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-02-10 0:16 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-08 21:42 [Possible Breakage] t1308 - Bad return value from test-tool Randall S. Becker
2019-02-09 4:24 ` Jeff King
2019-02-09 18:08 ` Randall S. Becker
2019-02-09 23:32 ` Jeff King
2019-02-10 0:15 ` Randall S. Becker
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).