[QA] UI testing

[jesmrec]

This issue is intended to be an epic to discuss about automatic testing.

Earlgrey is the chosen tool to develop UI tests. The branch earlgrey-login contains a first draft of a couple of tests.

We can face two different targets using Earlgrey:

• UI testing: check how the app reacts to the SDK's responses. Abstracting dependencies from SDK and mocking responses make posible to isolate the app. This kind of testing complements the unit test in the SDK to have a greater coverage through the full stack of the app.

• E2E testing: the scope is the whole app, by using the SDK itself. In this case the whole stack is tested, but is more difficult to abstract tests, and positive results will not give the accurate point of the issue/bug. By using a real server, network/core performance issues can happen causing wrong positives and making the tests flaky.

The point is that we need to abstract dependencies in the app for UI testing, by making testable code. Currently, the app calls directly the SDK interfaces to get info. We should be able to use the same calls with mocked objects, in a way similar to the following one:

1. Create mocked object(s)
2. Set the mocked object(s) with the desired response
3. Perform actions
4. The mocked SDK answers with the canned response in 2.
5. Check and assert what happens in UI

for each test. Options to achieve it:

• Put all dependencies as parameters. Therefore, by subclassing we can call the functions with mocks. Of course, this point has code design/structure implications.
• Use the SDK dependency as a mock.
• ... (more ideas here).

All additional and related ideas are welcome.

@felix-schwarz @javiergonzper @pablocarmu @michaelstingl

[pablocarmu]

Here it is a little summary of the discussions/thoughts @felix-schwarz and I had about how to make UI testing in the app more straightforward:

Host Simulator

There is a mechanism called Host Simulator in the SDK that intercepts previously selected network calls and mock the response for this particular call with a set response. There are examples of how to use this mechanism in some SDK tests, like here

The problem using the Host Simulator is that is going to be impossible to test particular app states in a scalable way.

Key-Value Observing

Set the values of the SDK objects directly to that they need to be to test the app. For read-only properties or private variables, we can leverage the -setValue:forKey: method from KVO.

Method Swizzling

Swizzle individual methods to replace them with versions to return the values needed for testing.

Modified SDK

Create a modified version of the SDK, taking advantage of the C preprocessor, like so:

// ModifiedOCQuery.m
// Turn OCQuery into BaseOCQuery
#define OCQuery BaseOCQuery
#import "OCQuery.m"
#undef OCQuery

// Subclass BaseOCQuery
@interface OCQuery : BaseOCQuery
@end

@implementation OCQuery

- (void)methodToOverride;

@end

Private ownCloud Server

Another thing that came up to my mind is to have a private ownCloud server only set up for app testing and, through some script, or OCcomands, leave the server in the state we want for the whole app testing or even for each specific test-suit.

These ideas are the result of some light discussions, there could be more suitable options than what is summarized in this comment. Also if I miss something or I've written something wrong @felix-schwarz you can correct me.

[jesmrec]

the options that seem to be more suitable are KVO and modified SDK, but both cases will need to develop testable code and estimate efforts.

The case of private oC server fits for the case of E2E testing, in which we are testing app + sdk, where the sdk is already covered with unit tests. We also lose the perspective of the app testing, so one failure can be caused in the sdk making wrong positives in the app.

[javiergonzper]

After check the options and see examples of swift projects provided by @pablocarmu (thanks!) IMHO the best solution it is:

• Use a mocked subclasses of the main SDK classes and overwrite their methods
• Use a typealias and a preprocessor macro on the ios-app to call the right classes (test subclasses or sdk classes, check the code at the end of the comment)
• Replace on the ios-app the calls to the SDK classes to the typealias

if @jesmrec @pablocarmu and @felix-schwarz agree with this implementation I can start with it 👍

#if os(macOS)
import AppKit

public typealias View = NSView
public typealias Color = NSColor
public typealias Image = NSImage
public typealias Screen = NSScreen
public typealias Font = NSFont
public typealias EdgeInsets = NSEdgeInsets

internal typealias ClickGestureRecognizer = NSClickGestureRecognizer
#else
import UIKit

public typealias View = UIView
public typealias Color = UIColor
public typealias Image = UIImage
public typealias Screen = UIScreen
public typealias Font = UIFont
public typealias EdgeInsets = UIEdgeInsets

internal typealias ClickGestureRecognizer = UITapGestureRecognizer
#endif

[felix-schwarz]

Here's a solution that enables mocking, but requires no changes to existing code (which is something I'd like to avoid TBH):

Create a new build configuration: ios-sdk Create a new build configuration "Mock" in the SDK that includes a prefix header that changes all names per #define:

#define OCCore MOCCore
#define OCQuery MOCQuery

That makes sure that the classes are actually called MOCCore and MOCQuery in the binary.

Create a new build configuration: ios-app Create a new build configuration "Mock" in the app. This makes sure Xcode builds and links the "Mock" version of the SDK.

Also, add a define in the build settings for this new configuration MOCK=1.

Add type aliases or mocking subclasses to ios-app In the ios-app, it's then possible to do this:

#if MOCK
typealias OCCore = MOCCore

class OCQuery : MOCQuery {
    // Subclass the methods you want to mock
}
#endif

Change ios-app project settings to use the Mock build configuration for testing To make sure, the mocking version is always used during testing, change the scheme in Xcode so that it uses the Mock build configuration.

--

Please let me know if you agree @pablocarmu @jesmrec @javiergonzper. I can make these changes relatively quickly then.

[javiergonzper]

@felix-schwarz for me it is ok 👍 it should works

[jesmrec]

WFM

[felix-schwarz]

I just tried to implement my suggestion but had to realize typealias and #defines won't do:

• MOCCore would still be creating and returning instances of MOCItems, even if the app subclasses MOCItem as OCItem. So the reach of subclassing here is generally limited to object the app creates itself. Which are almost none. Which disqualifies subclassing for mocking the SDK IMO.

• Even leaving that aside, Swift insists on type safety. There's no problem as long as a type is just typealiased, but as soon as a type is subclasses to its "original" name, Swift throws Cannot assign value of type 'MOCItem?' to type 'OCItem?'-like compilation errors all over the code base.

• serialization and deserialization of existing data creates errors as it can't find the classes that are merely typealiased

Ok, so with that approach (and typealiasing in general - for this purpose) off the table, what else could be done? I see three options:

1) insert mocking code directly into the SDK, enabled/disabled with #ifdefs. I would like to avoid this.

2) keep a "Mock" target and within that:

• make select methods "mockable" by providing a dictionary mapping a unique string ID (i.e. OCCore.startQuery) to a block (i.e. (core: OCCore, query: OCQuery) -> Void)
• add code specific to that target that swizzles the targeted methods and checks said dictionary for a block. If a block is found, it is executed. If none is found, the SDK implementation of that method is used.

3) keep a "Mock" target and within that:

• allow mocking by using extensions that implement special mocking versions of existing methods, i.e. func mock_startQuery(query: OCQuery).
• add code specific to that target that swizzles the targeted methods and checks for implementations of the mocking versions of existing methods. If one is found, it is used. Otherwise, the SDK implementation is used.

Personally, I'd avoid 1), consider 3), but likely go for 2) because it gives the most flexibility and possibly the cleanest code.

@javiergonzper @jesmrec @pablocarmu

[felix-schwarz]

The feature/sync branch of the SDK adds the new ownCloudMocking.framework.

Linking against it provides access to OCMockManager, which implements what I've been writing about above.

Check out https://github.com/owncloud/ios-sdk/blob/137792e14b888de51cb0eef45de7684d2f9a8f41/ownCloudMockingTests/ownCloudMockingTests.m for a simple example of how mocking works for this class: https://github.com/owncloud/ios-sdk/blob/137792e14b888de51cb0eef45de7684d2f9a8f41/ownCloudMockingTests/OCMockTestClass.h .

[javiergonzper]

I created the branch feature/mockTestFromSDK on the app and on the SDK with the code to make a an example test with mocking using swizzling. It is working 👍. Right now it could be an entry point to start mocking the UI tests mocking the necessary methods of the SDK to all the tests.

[javiergonzper]

I am happy to say that we have our first UI Tests on the app that mock the response of the server.

Right now we can develop the next test using as example the mock of the class OCConnection and the method prepareForSetup. I created a couple of PR one for the app and other for the SDK: ios-app: https://github.com/owncloud/ios-app/pull/136 ios-sdk: https://github.com/owncloud/ios-sdk/pull/30

Before merge the PR of the ios-app we need to fix the compilation issues on the branch feature/fileprovider

[javiergonzper]

After develop several tests we are discovering the limitations of EarlGrey:

The app is not rebooted after each test This is a problem because every test must return the app to the original View. If the test fails the view will not be restored to the original view so all the following tests probably will fail.

It is not possible to access to the real objects in the screen For example to get the number of cells or access a concrete cell in a table to get the title value.

I would recommend to start using Kif (https://github.com/kif-framework/KIF) and remove EarlGrey from the project.

If no one have any inconvenience about it I want to start to do the change as soon as possible CC: @jesmrec @michaelstingl @mneuwert @felix-schwarz @pablocarmu

[jesmrec]

The app is not rebooted after each test This is a problem because every test must return the app to the original View. If the test fails the view will not be restored to the original view so all the following tests probably will fail.

This is really scary. Each test must be independent from all other tests, and should start from a clean status of the app, not depending on the way they are executed. If tearDown does not reset completely the app, we are on the risk of having "binary" tests: tests that tell you either "all works" or "something does not work, but i'm not going to tell you what o where the it is". Of course, this is better than not having tests, but in case of having a huge amount of tests, it will not help.

Does KIF work in that way?

[felix-schwarz]

On relaunches / resets:

The EarlGrey FAQ explains:

EarlGrey is unable to launch or terminate the app under test from within the test case, something that Xcode UI Testing is capable of.

• Unlike Xcode UI tests, both KIF and EarlGrey base on XCTest, run inside the app itself and therefore neither KIF nor EarlGrey can relaunch the app.

• To me it seems best practice to navigate to a specific spot in the UI - and back out, to verify navigation works both ways. Navigation to a specific point in the app, and back to the "start screen", should be put in reusable methods anyway to keep the tests maintainable, so a typical test could look like this (pseudo-code):

  func testSomething() {
  self.createBookmark()
  self.loginToBookmark(atPosition: 0)
  
  // UI test
  
  self.logout()
  self.deleteLastBookmark()
  }

• Bonus idea: "building blocks" like loginToBookmark could automatically push blocks of code to an array, which can then be executed in reverse order to go back to the "start screen". The code could then be simplified like this:

  func testSomething() {
  self.createBookmark() // adds block with self.deleteLastBookmark() to array
  self.loginToBookmark(atPosition: 0) // adds block with self.logout() to array
  
  // UI test
  
  self.resetUI() // runs blocks in reverse order (might even move this into tearDown()).
  }

• In the event that one test fails and other, subsequent tests fail because of that, we can see in the logs which test failed first and for what reason. We can then work to fix the issue, so the test no longer fails. If fixing the test takes longer, we can temporarily rename it to start with something other than "test" so Xcode no longer executes it.

On accessing specific views:

• The EarlGrey FAQ, shows how to get UI elements, invoke methods on them and fetch method results, including in Swift.

On switching to KIF:

• PSPDFKit, which is heavily invested in UI testing recommends EarlGrey and considers KIF legacy code.
• Which rings true if you look at the GitHub pulse of both projects: there's a lot more activity in EarlGrey (last commit Nov 1st, adding support for WKWebView vs Aug, 23rd, adding code to avoid XCUIScreenshot in Xcode 10 beta(!)) – and it's created, backed and used by Google for their own apps.

Conclusion

I see no reason or advantage to switch to KIF and strongly recommend staying with EarlGrey.

CC: @javiergonzper @jesmrec @michaelstingl @mneuwert @pablocarmu

[jesmrec]

Unlike Xcode UI tests, both KIF and EarlGrey base on XCTest, run inside the app itself and therefore neither KIF nor EarlGrey can relaunch the app.

To me it seems best practice to navigate to a specific spot in the UI - and back out, to verify navigation works both ways. Navigation to a specific point in the app, and back to the "start screen", should be put in reusable methods anyway to keep the tests maintainable, so a typical test could look like this (pseudo-code):

more that relaunching the app, we need to launch every test from scratch. I guess KIF allows to restart the app (not reinstalling), so every test will start from the same point, regardless of the status in which the previous test finished. This is that Earlgrey lacks, and make all test fails after the first failure.

From QA pov, each test should be as much scoped as posible. Browsing through views before/after the main actions will increase the risk of failures that are no caused by the target of the test, and then, not-desirable false positives.

In the event that one test fails and other, subsequent tests fail because of that, we can see in the logs which test failed first and for what reason. We can then work to fix the issue, so the test no longer fails. If fixing the test takes longer, we can temporarily rename it to start with something other than "test" so Xcode no longer executes it.

i think that tests are developed to avoid such things. One test is intended to cover one scenario. When the test fail, you know that something is broken in the scenario and then, research about it to get the bug or whatever.

Discussing about EarlGrey or KIF (or other one) is the best, does not have place. We should choose the best for our necessities.

I will not deep dive in the technical stuff, so i trust in all of you, but what i wrote here https://github.com/owncloud/ios-app/issues/58#issuecomment-439016099 needs to be studied because is one of the first premises to get good and optimal tests.

[felix-schwarz]

more that relaunching the app, we need to launch every test from scratch. I guess KIF allows to restart the app (not reinstalling),

I don't see how KIF should pull that off, as it runs inside the app itself. As soon as it terminates the app, it terminates itself and can no longer execute code.

The KIF developers say pretty much the same:

• https://github.com/kif-framework/KIF/issues/694#issuecomment-283955138

KIF runs in a unit test target, not a UI test. If one test fails, the app will likely not be in a testable state at that point, and it makes sense for the tests to stop.

• https://github.com/kif-framework/KIF/issues/694#issuecomment-413142017

You can't fully restart the app in the way that XCUI does, because in that framework the tests are running in a separate process space from your application.

so every test will start from the same point, regardless of the status in which the previous test finished. This is that Earlgrey lacks, and make all test fails after the first failure.

There's no difference between KIF and EarlGrey in this regard.

From QA pov, each test should be as much scoped as posible. Browsing through views before/after the main actions will increase the risk of failures that are no caused by the target of the test, and then, not-desirable false positives.

I agree. However, as long as we want tests to run inside the apps themselves, so we can also verify internal state, there's really no alternative to this.

Xcode UI tests would supposedly allow relaunching the app between tests, but run in a separate process. Introspection and verifying internal state (like @javiergonzper said he wants to do with row counts of table views if I understood correctly) is then no longer possible.

In the event that one test fails and other, subsequent tests fail because of that, we can see in the logs which test failed first and for what reason. We can then work to fix the issue, so the test no longer fails. If fixing the test takes longer, we can temporarily rename it to start with something other than "test" so Xcode no longer executes it.

i think that tests are developed to avoid such things. One test is intended to cover one scenario. When the test fail, you know that something is broken in the scenario and then, research about it to get the bug or whatever.

It's certainly desirable, but not available with frameworks like EarlGrey or KIF.

As I wrote above, Xcode UI tests are the only ones offering this, but will limit the tests purely to UI elements.

Discussing about EarlGrey or KIF (or other one) is the best, does not have place. We should choose the best for our necessities.

Yeah. That's my point, too.

I will not deep dive in the technical stuff, so i trust in all of you, but what i wrote here #58 (comment) needs to be studied because is one of the first premises to get good and optimal tests.

There's no difference between KIF and EarlGrey in this regard.

[javiergonzper]

I am trying to add more test for the login view but I am having problems due the coupled code between the BookmarkViewController and the OwncloudSDK.

I see that the BookmarkViewController create an object OCBookmark that is passed to the OwncloudSDK when is created the object connection of OCConnection. During the execution of connection.prepareForSetup the initial object OCBookmark change its properties inside the SDK and then that it is reflected on the BookmarkViewController.

Things like that make the project difficult to understand and maintain and of course extremely difficult to test.

Is it possible to avoid those kind of practices?

CC: @michaelstingl @jesmrec @felix-schwarz

[jesmrec]

From a QA pov, the most desirable property is the module isolation, so we can test app and SDK independent each other by mocking, making tests more accurate, useful and maintainable. Please, take it in account in development stage, so mocks will be developed easily and coverage will increase faster.

Any solution for the problem @javiergonzper mentioned?

[javiergonzper]

After add several UI Tests using Swizzling to mock the responses of SDK methods I found a big problem:

The SDK do not response always on the method called the result of a query sent the ownCloud Server. Everything related with the list of files is receive in a Delegate

Example

1. APP -> call to the SDK OCCore.createFolder
2. APP -> do not receive the response immediately inside the method call (sync or async with a completion)
3. SDK -> send the http requests to the ownCloud Server
4. SDK -> receive the response of the ownCloud Server
5. SDK -> process the response updating information on Database
6. SDK -> Call to APP OCQueryDelegate method with the result of the Query (error, new items...)

Problem

The problem using Swizzling is that we mock the first call on 1. Step to OCCore.createFolder but there we do not receive the response. The response is received on the OCQueryDelegate on 6. step. So mock OCCore.createFolder do not help.

Remember that when we make Swizzling to a method we change the internal implementation. All the things that the original method make are not executed.

I talked with @felix-schwarz about this issue and he propose mock the http requests using OCHostSimulator. It is something very similar to Nocilla library. The idea it is simulate the response of a server when a request it is made to a concrete resource (stubbing). For example if we make a GET to URL http://demo.owncloud.com/ with HEADER XXX... we stub the response returning whatever we want (404, 200 with an XML in the body...).

Conclusion

The problem of make Stubbing using OCHostSimulator or Nocilla:

• The SDK make several request to make anything. So it is very difficult to scale.
• To make any action we will need to make a complete process. (stubbing the login, the list of files, the creation of the folder... and so on)
• This approach will not allow us to create atomic "UI Test". We will test the whole the process of the SDK so if the SDK have an issue we will have a failure on the UI Tests. For example a testCreationFolder could fail because the the SDK fails inserting the user name.

cc: @jesmrec @pablocarmu @felix-schwarz @michaelstingl @mneuwert

[jesmrec]

This is really a point i did not want to reach.

If we go to the OCHostSimulator or Nocilla approach, we are transforming UI tests (or tests along the app) to something that will not improve the quality as expected, only covers user scenarios and let you know if something in the middle is wrong, but not the specific error . Every test must have an specific scope, and must be isolated from other dependencies, this is the basis of testing. If i want to test the “Move file": i must create the bookmark, browse, upload the file i want to move, create the target folder, and then execute the action… this is too much noise that will make the tests spend triple or more time to run, and increasing exponentially the odds of wrong positives. Of course, the scenarios can be simplifed (no browse, using skeleton files...). but it would reduce the coverage.

Nocilla,OCHostSimulator, and other similar tools are intended to fake dependencies when the test object is linked directly to the network, and this is not the case of ios-app

I really hope that a better solution can be found here.

[felix-schwarz]

I do not share these conclusions or fears. Here's why:

We have tools to match every need

• OCHostSimulator: to simulate server responses and the measure/check the reaction along the whole SDK+app stack.
• OCMocking: to replace specific methods in the SDK with mocked implementations.
• subclassing: for everything else

By the example

Going through the example:

Preparations

• Subclass OCCore as MockCore
• Subclass OCQuery as MockQuery, so that an array of items (mockItems ?) on an instance:
  • calls the query's delegate queryHasChangesAvailable() method
  • provides the provided mockItems when the MockQuery's requestChangeSet(withFlags:completion:) method is called in the OCQueryChangeSet returned via the completion-block of that MockQuery method
• Bring up ClientQueryViewController with instances of MockCore and MockQuery

From this point on, you can replace all calls to the Core and Query with your own in the subclasses.

Implementation

Example

1. APP -> call to the SDK OCCore.createFolder

This goes to your own implementation in MockCore.createFolder. Suggestions on what it can do below.

2. APP -> do not receive the response immediately inside the method call (sync or async with a completion)
3. SDK -> send the http requests to the ownCloud Server
4. SDK -> receive the response of the ownCloud Server
5. SDK -> process the response updating information on Database

These steps are SDK-internal and - following Jesus' requirement to have isolated tests and best practices - can be skipped.

6. SDK -> Call to APP OCQueryDelegate method with the result of the Query (error, new items...)

MockCore.createFolder calls the completionHandler with the desired result and modifies the items in the MockQuery, so it triggers all the callbacks and provides the modified items (see preparations above for how to implement this in a few lines of code).

cc: @jesmrec @pablocarmu @javiergonzper @michaelstingl @mneuwert

[felix-schwarz]

Let me add, too, that all actions - including create folder - are encapsulated in extensions by now, and therefore usable on their own. That's makes even more focused tests possible.

cc: @jesmrec @pablocarmu @javiergonzper @michaelstingl @mneuwert

[javiergonzper]

Right now the only way to use the MockCore or the MockQuery is checking if we are on Testing Mode to init a OCore or MockCore.

For example:

if IS_TESTING {
    core = MockCore.shared.requestCore(for: bookmark, completionHandler: { (_, error) in
        if error == nil {
            self.coreReady()
        }

        openProgress.localizedDescription = "Connected.".localized
        openProgress.completedUnitCount = 1
        openProgress.totalUnitCount = 1

        self.progressSummarizer?.stopTracking(progress: openProgress)
    })
} else {
    core = OCCoreManager.shared.requestCore(for: bookmark, completionHandler: { (_, error) in
        if error == nil {
            self.coreReady()
        }

        openProgress.localizedDescription = "Connected.".localized
        openProgress.completedUnitCount = 1
        openProgress.totalUnitCount = 1

        self.progressSummarizer?.stopTracking(progress: openProgress)
    })
}

Is it right for you? I do not like it because it is very ugly add if TESTING on the ViewControllers inside the app.

To do not add if TESTING in several places I spoke with @pablocarmu and the only possibility that we see is modify the architecture of the app to create the Views inyecting them the necessary objects depending if we are in mock mode or not.

What do you think?

If you have a better idea, please create full example of a real test for us in order to follow it.

[javiergonzper]

Right now as talk on one of our last call I am trying to Subclass the OCCore and OCQuery to create a simple test of creation of a folder.

[javiergonzper]

Using a subclass of OCCore and OCQuery I can show a ClientQueryViewController. That is good because I can update the UI of the ClientQueryViewController calling to the delgate: query?.delegate.queryHasChangesAvailable(query!)

Thanks to that I implemented a test of a creation of the folder 👍

Now the problem that I have is that if I just show ClientQueryViewController I lost the OCCoreDelegate that it is on the ClientRootViewController so I can not manage the error. Maybe I can show the full UI of the file list and then using Swizzling in some point call to the OCCoreDelegate

[javiergonzper]

Subclassed:

• MockOCQuery
• MockOCCore
• MockClientRootViewController

With those change now we can show the file list with the ClientRootViewController (tabs and navigation bar)

I added 6 tests about "Create folder"