StorageTests are disabled and broken

RESOLVED FIXED

Status

()

Firefox for iOS
Build & Test
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Sachin Irukula, Assigned: bnicholson)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

3 years ago
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_2) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.101 Safari/537.36

Steps to reproduce:

1 . Checkout a the trunk
2. Follow the instructions to build.


Actual results:

When I try to build the Client an error Missing argument for parameter 'rootPath' in call is throw in some of the files 
firefox-ios/StorageTests/SQLiteRemoteClientsAndTabsTests.swift
firefox-ios/StorageTests/TestTableTable.swift etc.

I think the problem is when MockFiles is instantiated it requires a rootPath parameter which is not being passed currently. MockFiles inherits from FileAccessor whose initializer takes rootPath as parameter.


Expected results:

Build should have been a success.

Updated

3 years ago
Flags: needinfo?(nalexander)
(Reporter)

Comment 1

3 years ago
I did some more digging and found out that this started happening after the commit https://github.com/mozilla/firefox-ios/commit/82fb582cd79666bd866b14de60766d1cf481f4c9

In this commit init method has been added for superclass FileAccessor and remove method parameters have been changed and similar changes should happen in the subclass MockFiles which didn't.
(Assignee)

Comment 2

3 years ago
You probably need to clear your DerivedData -- StorageTests aren't running at all right now, so they shouldn't be failing in a working build.

That said, StorageTests shouldn't be disabled in the first place. StorageTests were part of the Storage framework, which was deleted in bug 1145335. Looks like we forgot to move them into the Client scheme.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
Flags: needinfo?(nalexander)
Resolution: --- → FIXED
Summary: Missing argument for parameter rootPath in call → StorageTests are disabled and broken
(Assignee)

Comment 3

3 years ago
Oops, didn't mean to close this yet.
Status: RESOLVED → REOPENED
Ever confirmed: true
Resolution: FIXED → ---
(Assignee)

Comment 4

3 years ago
Created attachment 8581717 [details]
Pull request
Assignee: nobody → bnicholson
Status: REOPENED → ASSIGNED
Attachment #8581717 - Flags: review?(nalexander)
Comment on attachment 8581717 [details]
Pull request

sachin: thanks for the report and the sleuthing!  This was all my fault.  bnicholson, patch looks good and tests pass on simulator locally.  (And ClientTests does include StorageTests now.)
Attachment #8581717 - Flags: review?(nalexander) → review+
(Assignee)

Updated

3 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.