Opened 8 years ago

Closed 8 years ago

#1845 closed defect (fixed)

SQLite3Update.rollbackFailure test fails

Reported by: vorner Owned by: jelte
Priority: very high Milestone: Sprint-20120417
Component: data source Version:
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DNS Feature Depending on Ticket:
Estimated Difficulty: 4 Add Hours to Ticket: 0
Total Hours: 0.16 Internal?: no

Description

The test fails for me:

[ RUN      ] SQLite3Update.rollbackFailure
sqlite3_accessor_unittest.cc:784: Failure
Expected: accessor->rollback() throws an exception of type DataSourceError.
  Actual: it throws nothing.
[  FAILED  ] SQLite3Update.rollbackFailure (10 ms)

I'd like this fixed soon, if possible, as I need to filter it out when running the tests.

Subtickets

Change History (16)

comment:1 Changed 8 years ago by jreed

On the debian-sid-64-4 system this bug was introduced with no BIND 10 code changes when package upgrade happened:

Start-Date: 2012-03-27  12:38:20
Commandline: apt-get install sqlite3
Install: sqlite3:amd64 (3.7.11-2)
Upgrade: libsqlite3-dev:amd64 (3.7.9-2, 3.7.11-2), libsqlite3-0:amd64 
(3.7.9-2, 3.7.11-2)
End-Date: 2012-03-27  12:38:27

comment:2 Changed 8 years ago by jreed

I disabled the test in src/lib/datasrc/tests/sqlite3_accessor_unittest.cc in commit 6db3556488ae8a75f5250a175f3507cc609ada19

comment:3 Changed 8 years ago by jinmei

It appears to be due to a behavior change in SQLite3 3.7.11:
http://sqlite.org/releaselog/3_7_11.html

Pending statements no longer block ROLLBACK. Instead, the pending
statement will return SQLITE_ABORT upon next access after the
ROLLBACK.

The change itself seems to be reasonable as ROLLBACK shouldn't easily
fail. But unless it ensures it never fails (which is not clear from
the SQLite3 doc) it's better to have a test case for that. And, as a
matter of fact, we are also using older versions of SQLite3 where
ROLLBACK would fail due to a pending statement.

My suggestion is:

  • Try to find another reliable way to make ROLLBACK fail and use it in this test.
  • If it's not possible or is very difficult, enable this test only for older versions of SQLite3 (<= 3.7.10).

comment:4 Changed 8 years ago by jinmei

  • Priority changed from medium to very high

comment:5 Changed 8 years ago by jelte

  • Milestone changed from Next-Sprint-Proposed to Sprint-20120417

comment:6 Changed 8 years ago by jelte

  • Owner set to jelte
  • Status changed from new to assigned

comment:7 follow-up: Changed 8 years ago by jelte

  • Owner changed from jelte to UnAssigned
  • Status changed from assigned to reviewing

I can't find anything in the documentation that suggests another failure mode for rollback, so the choice would be to remove the test altogether, or do a version check. Opted for the latter.

comment:8 in reply to: ↑ 7 Changed 8 years ago by jinmei

Replying to jelte:

I can't find anything in the documentation that suggests another failure mode for rollback, so the choice would be to remove the test altogether, or do a version check. Opted for the latter.

It basically looks okay.

One possible change I might suggest is to still run the test for both
types of behaviors, just with different expectations:

#if SQLITE_VERSION_NUMBER < 3007011
    EXPECT_THROW(accessor->rollback(), DataSourceError);
#else
    EXPECT_NO_THROW(accessor->rollback(), DataSourceError);
#endif

I'd leave it to you.

comment:9 Changed 8 years ago by jinmei

  • Owner changed from UnAssigned to jelte

comment:10 Changed 8 years ago by jinmei

  • Total Hours changed from 0 to 0.1

comment:11 follow-up: Changed 8 years ago by jelte

  • Owner changed from jelte to jinmei

Hmm, good point, and come to think of it, we should probably also check how the 'running transaction' behaves when there is a rollback (the change being that the rollback no longer fails, but the next action will with an ABORT message). So I also added a check for (no) throw on iterator->getNext().

comment:12 in reply to: ↑ 11 Changed 8 years ago by jinmei

Replying to jelte:

Hmm, good point, and come to think of it, we should probably also check how the 'running transaction' behaves when there is a rollback (the change being that the rollback no longer fails, but the next action will with an ABORT message). So I also added a check for (no) throw on iterator->getNext().

The code looks okay to me. But the sqlite3 version on my dev
environment is still old so I cannot confirm the behavior. If you
actually checked the new behavior after ROLLBACK, please merge. If
your version is also old...hmm, maybe we should merge it and check the
behavior on the buildbot that triggered this.

comment:13 Changed 8 years ago by jinmei

  • Owner changed from jinmei to jelte

comment:14 Changed 8 years ago by jinmei

  • Total Hours changed from 0.1 to 0.16

comment:15 Changed 8 years ago by jinmei

I confirmed the test passed on the test box that installs SQLite3 3.7.11.

So the branch is okay for merge.

comment:16 Changed 8 years ago by jelte

  • Resolution set to fixed
  • Status changed from reviewing to closed

done, closing ticket.

Note: See TracTickets for help on using tickets.