489481 - Get C++ tests for our C++ helper classes

Status: RESOLVED FIXED

(Toolkit :: Storage, defect)

Description

[Shawn Wilsher :sdwilsh]

I'd like to see our C++ helper objects and our noscript methods tested with some C++ tests.

Comment 1

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

Also fixes a bug found in mozStorageTransaction.  I don't think anybody was really depending on it's currently useless behavior...

Comment 2

[Shawn Wilsher :sdwilsh]

Comment on attachment 374341 [details] [diff] [review]
v1.0

I seem to have actually forgotten to request review here, but I'm going to switch to vlad in hopes of freeing up asuth a bit.

vlad - this is mostly new tests and getting rid of an old unused test (that has coverage with xpcshell).  There are some minor code changes that were required due to bugs found while writing the tests.

Comment 3

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 374341 [details] [diff] [review]
v1.0

but storage1's name was so catchy!

I get warnings like so:
storage_test_harness_tail.h:60: warning: ISO C++ does not support the ‘z’ printf length modifier

Don't know if they will make bsmedberg's warning detector angry at you.  Arguably you could just use an unsigned int for gTotalTests/gPassedTests and lose the 'z' specifier.

nit: s/innterTransaction/innerTransaction/
(no 't')

Comment 4

[Shawn Wilsher :sdwilsh]

(In reply to comment #3)
> I get warnings like so:
> storage_test_harness_tail.h:60: warning: ISO C++ does not support the ‘z’
> printf length modifier
hrm - I didn't get those.  What platform/version of gcc are you on?

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

(In reply to comment #4)
> hrm - I didn't get those.  What platform/version of gcc are you on?

Ubuntu 9.04 amd64
g++ (Ubuntu 4.3.3-5ubuntu4) 4.3.3

Comment 6

[Shawn Wilsher :sdwilsh]

Attachment: v1.1

Addresses review comments.  Also fixed the license block which attributed this to Oracle still (oops).

Comment 7

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/9c62b2169f76

Nobody seemed to be too worried about the %z for the printf on irc.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

Yeah, I don't care about the warning either, just that the automated warning tool might get on your case.