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