BIND 10 #1895: regression fix: set B10_FROM_BUILD for auth config tests
BIND 10 Development
do-not-reply at isc.org
Tue Apr 17 16:26:02 UTC 2012
#1895: regression fix: set B10_FROM_BUILD for auth config tests
-------------------------------------+-------------------------------------
Reporter: | Owner: jinmei
jinmei | Status: reviewing
Type: | Milestone:
defect | Sprint-20120501
Priority: very | Resolution:
high | Sensitive: 0
Component: build | Sub-Project: DNS
system | Estimated Difficulty: 0
Keywords: | Total Hours: 0
Defect Severity: N/A |
Feature Depending on Ticket: |
Add Hours to Ticket: 0 |
Internal?: 0 |
-------------------------------------+-------------------------------------
Comment (by jinmei):
Replying to [comment:3 jelte]:
> Why not set environment variable for tests directly, like we do for a
number of other tests?
Because I wanted to make it workable even if we invoke the executable
directly, maybe for specifying gtest filter. We could introduce a
separate wrapper shell script, but it seemed to be more error prone
(we can easily overlook the need for the script).
> If there is a legit reason to do this from within the test code itself,
a few small comments:
After reading the comment, I updated my mind a bit and switched to
setting this variable globally for the entire tests. At least it
doesn't break any other tests right now, and I guess we'll also need
it for other cases pretty soon (at least for supporting the reload
command for inmem-over-sqlite3, and eventually other general cases
when we enable the factory).
I've clarified these points as comments in main().
So the following points may become moot now, but responding to them
anyway...
> - since followup may be borked, EXPECT_EQ should probably be ASSERT_EQ
in TearDown (though it won't matter in practice, since it is already done,
it would be slightly more indicative of the seriousness :)
Hmm, is ASSERT_EQ better in this case? So we don't try to restore the
rest of the variables? I do not necessarily object to that, but am
not sure if that's definitely better than EXPECT_EQ (in which case we
leave a warning for the failure case but still try to do the best
restoring as many as possible). Since failure of this is something
very unexpected, if we worry about the seriousness I'd rather assert()
that.
> "Couldn't restore environment, results of other tests" and "are
uncertain"; need a space at the end of the first or the start of the
second string
Ah, right. It was actually an issue of the code in common_unittest
that I copied for this branch. I've fixed the original part.
--
Ticket URL: <http://bind10.isc.org/ticket/1895#comment:5>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list