Closed Bug 981943 Opened 7 years ago Closed 7 years ago

Clock Database Unit Test failure when running in B2G Desktop with OOP

Categories

(Firefox OS Graveyard :: Gaia::Clock, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
1.4 S3 (14mar)

People

(Reporter: qdot, Assigned: qdot)

References

Details

(Whiteboard: [systemsfe] [p=5])

Attachments

(1 file)

I'm working on getting it so we can turn on OOP in B2G Desktop on linux, and keep getting failures in the clock app. After running the "Get a connection fails with no schemas" test, the teardown function is getting onblocked back, which causes an error. Since gaia is green on travis at the moment, this seems to only be an issue on OOP.
Adding a database close to the failure path seems to fix this. That said, I have NO idea why this only happens in OOP, seems like it should happen in-process too? Also, found an event name typo that I fixed.
Attachment #8388920 - Flags: review?(m)
Summary: Clock Database Unit Test failure when running in B2G Desktop with OOP on → Clock Database Unit Test failure when running in B2G Desktop with OOP
Attachment #8388920 - Flags: review?(m) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #1)
> Created attachment 8388920 [details] [review]
> Patch 1 (v1) - Add database close to upgrade failure path in clock database
> 
> Adding a database close to the failure path seems to fix this. That said, I
> have NO idea why this only happens in OOP, seems like it should happen
> in-process too? Also, found an event name typo that I fixed.

You should ask bent :)
Consulted bent on this before I talked to the clock people actually. He was confused too. There's a lot happening in clock's database access library.
In what I can only describe as a most unfortunate admission, I have bug 977692 in progress to refactor Clock stuff. Bocoup recently handed over Clock to us, and it was only in this refactoring last week (after Kyle had pinged me on IRC) that I realized this file isn't actually used anywhere. There are unit tests, but we don't use this database. I intend to remove it when the refactor lands.

So it's maybe good to see that this happened, but yeah, unfortunate that this code is still there even though it isn't actively used.
The ultimate goal was to move the database code to `shared/`, but we needed to "prove" it from within Clock first. As is often the case, bugs with more user-facing impact superseded this infrastructural work, so it never quite made it. I know Eric worked hard on this, so I want to rope him in on this conversation--maybe he should follow up in bug 977692.
Flags: needinfo?(eric)
Thanks for the bug fix :) 

As Mike said, it's likely that this should have been library code, but Gaia doesn't really have a third party library workflow, so we ended up going this roundabout way of trying to integrate it into the clock application to justify its position in shared/. 

Since most of the applications in Gaia are running just fine with the thin async storage shim on top of IndexedDB, it probably makes sense to remove it. I may pull this out into an Apache licensed project, because I believe it does make working with IndexedDB version changes much nicer, especially when testing out database changes.
Flags: needinfo?(eric)
Whiteboard: [systemsfe]
Target Milestone: --- → 1.4 S3 (14mar)
Whiteboard: [systemsfe] → [systemsfe] [p=5]
You need to log in before you can comment on or make changes to this bug.