BIND 10 #2480: use real data sources in auth QueryTest

[BIND 10 Development]

#2480: use real data sources in auth QueryTest
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  defect        |  jinmei
            Priority:  medium        |                       Status:
           Component:  b10-auth      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130108
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------

Comment (by jinmei):

 Thanks for the review.

 Replying to [comment:12 vorner]:

 > It doesn't compile for me, it seems the set of warnings of clang++ and
 g++ are
 > different. I think most of the defined but not used ones is side-effect
 of the
 > previous error (whenever there's an error in a test file, g++ spits out
 bunch
 > of unrelated warnings), the real one would be the control reaches…:

 Ah, okay.  I've reproduced it on a different machine, and fixed it.  I
 believe it works for you too.

 > Then, I think this kind of rules in Makefile are wrong:
 > {{{
 > testdata/example-base.zone example_base_inc.cc:
 testdata/query_testzone_data.txt
 >        $(PYTHON) $(srcdir)/gen-query-testdata.py \
 >                $(srcdir)/testdata/query_testzone_data.txt \
 >                testdata/example-base.zone example_base_inc.cc
 > }}}
 >
 > Make doesn't really know multi-target rules.
 [...]
 > If I look at the file `src/bin/auth/tests/testdata/example.zone.in`,
 would it
 > be better to use the generated directly, instead of a file that contains
 only
 > include of it?
 [...]
 > Thinking about it, could it be directly a master file and #var= would be
 ;var=?
 > In which case, only the .cc and sqlite3 files would need to be
 generated.

 I think these are all related, and your suggestion in the last should
 address all of the issues.  I updated the branch based on the
 suggestion: the source data are now in zone file format and
 multi-target rules are gone.  We might still avoid include-only top
 zone files, but include will be used for other purpose in a later
 change (see below).  So it's still kept.

 > Why is this safety-catch removed? It was supposed (I guess) to catch bad
 > updates to the test data:
 > {{{#!diff
 > -        const string hlabel = hash_map_[name.split(i, labels - i)];
 > -        if (hlabel.empty()) {
 > -            isc_throw(isc::Unexpected, "findNSEC3() hash failure for "
 <<
 > -                      name.split(i, labels - i));
 > -        }
 > +        const string hlabel = nsec3_hash_.calculate(name.split(i,
 labels - i));
 > }}}

 Ah, maybe I should note it explicitly, but nsec3_hash_ internally
 performs this check.

 > The test data contains two „var=“ (without the variable name) in a row,
 could
 > the second one be omitted?

 It could.  But I've kept it with some additional notes in comments
 including this point.  This point is a bit tricky so it should
 be worth commenting anyway.  We can still remove the
 technically-unnecessary "var=" markup if you want.

 > Also, about the way the tests are selectively disabled, I don't really
 like
 > this much. This looks like the thing is tested and passes (because the
 test is
 > listed in the output), but it is actually not tested. Wouldn't there be
 a way to
 > run only the right tests?

 Okay, I tried to improve the situation.  I introduced a derived test
 class that only uses the mock data source.  I also updated some other
 test cases that excluded in-memory due to the need for adding RRs
 dynamically, at the cost of additional setup complexity.

 This leaves cases that are disabled due to actual bugs of the
 corresponding data sources.  At least for in-memory I expect they'll
 be fixed and re-enabled soon (#2585).  If the SQLite3 case is really
 difficult as I expected in #2586, they'll remain disabled this way
 until they are fixed, but there are only two cases, so I think it's
 acceptable.

-- 
Ticket URL: <http://bind10.isc.org/ticket/2480#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development