424402 - Test a combination of shared and unshared database connections

Status: RESOLVED FIXED

(Toolkit :: Storage, defect, P2)

Description

[Shawn Wilsher :sdwilsh]

We currently don't have any tests in the tree that ensure that using a combination of shared and unshared database connections works as expected.  We *really* need this, as this issue was brought up in bug 423273.

Myk - I don't have any time for this, would you be willing to pick it up?

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

--> final milestone; I'm assuming since Schrep blocked at P2 this wasn't meant to block beta. If it was, set TM to beta5 and P1

Comment 2

[Mike Schroepfer]

Yes - set P2 for exactly that reason!

Comment 3

[Ondrej Brablc]

Attachment: Simple test

So what is expected when we combine shared cache and private cache databases? My understanding is, that from outside, we should not be able to see any difference.

This simple test opens two shared cache and two private cache connections to the same database and does some operations which do not lead to NS_ERROR_FILE_IS_LOCKED. But it does not prove anything, I'm afraid.

Comment 4

[Shawn Wilsher :sdwilsh]

The trick really is virtual tables work on a shared connection, and do not on an unshared one, so we should make sure that is the case, and somehow test that our turning the shared cache on and off is tested to ensure that it behaves correctly.

Comment 5

[Ondrej Brablc]

Attachment: Test switching opening in shared / unshared mode

Comment 6

[Ondrej Brablc]

Comment on attachment 311763 [details] [diff] [review]
Test switching opening in shared / unshared mode

I was too fast previously.

The test opens 4 databases, in different sharing modes: shared, private, shared, private. It tries creating virtual tables on them and expect failure on shared ones. It tests creation of normal table too.

Comment 7

[Shawn Wilsher :sdwilsh]

Comment on attachment 311763 [details] [diff] [review]
Test switching opening in shared / unshared mode

>+ * Portions created by the Initial Developer are Copyright (C) 2007
2008 ;)

>+const CONN =
>+{
>+  S1 : true,
>+  P1 : false,
>+  S2 : true,
>+  P2 : false
nit: no space after [S|P][0-9]

>+TestBody.prototype.cleanUp = function cleanUp()
note: should probably clean up before and after the test.  It looks to me like you could throw during the test and then not cleanup.

>+function run_test()
>+{
>+  var tb = new TestBody;
>+
>+  try {
>+    for (var prop in tb)
>+      if (prop.match(/^test_/) && typeof(tb[prop]) == "function")
>+        tb[prop]();
are we counting on the tests to be ran in the order they are declared, or counting on alphabetical order?  Either way - seems fragile.  I'd much prefer to just do an array of the functions to call like we do in all the other storage tests (in this case, it'd be the array of properties.

>+    tb.cleanUp();
oh, I see - you call cleanup here at the end of the test regardless, right?

Overall, this test could use a lot more comments.  If this test were to fail for someone, I don't expect them to figure out what's wrong and why very quickly at all.

Comment 8

[Ondrej Brablc]

Attachment: Test switching opening in shared / unshared mode v2

Fixed nits, added comments, lines cut to 80 characters.

I have changed the way the test methods are invoked (although Firefox seems to enumerate properties in the order of their insertion). However, I have avoided listing them. I find it very useful not to do this, because it assures, that there are no unused tests in the code.

Comment 9

[Ondrej Brablc]

Attachment: Test switching opening in shared / unshared mode v3

I have replaced the numbered tests with array of function names as you have requested. The numbers were not fragile but if someone would want to add a function in the middle, he would have to rename all the methods.

Comment 10

[Ondrej Brablc]

Comment on attachment 313915 [details] [diff] [review]
Test switching opening in shared / unshared mode v3

It looks like Shawn is too busy, can you mike make a review?

Comment 11

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 313915 [details] [diff] [review]
Test switching opening in shared / unshared mode v3

r+a=shaver, but please rename CONN to CONN_IS_SHARED or something similar, so that the tests can look like

if (CONN_IS_SHARED[conn])

instead of

if (CONN[conn]) // if it's shared

Thanks for writing these!

Comment 12

[Ondrej Brablc]

Attachment: Test switching opening in shared / unshared mode v4 (for check-in)

I cannot check-in myself, so please use this attachment instead. I have fixed readability problem and 80 character lines limit.

Comment 13

[Ondrej Brablc]

Please use attachment #316709 [details] [diff] [review] (v4) for check-in.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

mozilla/storage/test/unit/test_storage_combined_sharing.js 	1.1