New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit Tests No Longer Launch App Fully #7047

Merged
merged 6 commits into from Apr 19, 2017

Conversation

@astralbodies
Copy link
Member

astralbodies commented Apr 18, 2017

Closes #7046

This PR disconnects the WordPressAppDelegate when running the unit tests. This means that none of the app is really brought up into a running state (Analytics, Core Data, Reachability, etc). It results in both a quicker launch time for tests and also totally breaks any dependencies tests have on the state of the app as a whole.

The TestingAppDelegate class replaces WordPressAppDelegate inside of main.m when unit tests are running (specifically when the test bundle is injected and that class exists). The old main UI XIBs were deleted and replaced with an instantiation of the main UIWindow in the regular app delegate.

Testing

  1. Launch the app as usual in both iPhone & iPad simulators/devices.
  2. Make sure rotations work as expected, backgrounding, multitasking, etc. Just want to ensure the UIWindow creation isn't missing any settings that the XIBs may have set.
  3. Kill the app and launch the unit tests. Make sure they all pass.

Things to Note

I'm seeing a bunch of constraint break warnings in the console but I'm not 100% sure this PR didn't introduce them. @frosty & @kurzee you may want to take a look as I've seen split VC mentioned in the warnings.

@jleandroperez I had to update the notifications manager test to bring up an instance of Helpshift to test the class. Make sure I didn't break anything or am giving the test false positive results. 馃槃

Needs review: @frosty, @kurzee, @jleandroperez, @koke

Special Thanks

@jonreid and his App Delegate testing post.

astralbodies added some commits Apr 18, 2017

@astralbodies astralbodies added this to the 7.5 milestone Apr 18, 2017

@astralbodies astralbodies self-assigned this Apr 18, 2017

@astralbodies astralbodies requested review from frosty , koke , jleandroperez and kurzee Apr 18, 2017

@astralbodies astralbodies changed the title Unit Tests No Longer Launch App Fully in Simulator Unit Tests No Longer Launch App Fully Apr 18, 2017

@SergioEstevao

This comment has been minimized.

Copy link
Contributor

SergioEstevao commented Apr 18, 2017

@astralbodies really liked the way you implemented this!

@kurzee

kurzee approved these changes Apr 18, 2017

Copy link
Contributor

kurzee left a comment

I'm seeing a bunch of constraint break warnings in the console but I'm not 100% sure this PR didn't introduce them. @frosty & @kurzee you may want to take a look as I've seen split VC mentioned in the warnings.

There's some known breaking constraints coming from Reader cells, and Post list cells. It should be those?

@astralbodies everything else looks good!

@koke

koke approved these changes Apr 19, 2017

Copy link
Member

koke left a comment

:midnblown:

鉂わ笍 this. If anything, I'd suggest we tweak the blank root VC on the TestingAppDelegate so the simulator doesn't just show a black screen during unit tests

@property (nonatomic, strong, readonly) Reachability *internetReachability;
@property (nonatomic, strong, readonly) Reachability *wpcomReachability;
@property (nonatomic, assign, readonly) BOOL connectionAvailable;
@property (nonatomic, strong, readonly) WPUserAgent *userAgent;

This comment has been minimized.

@jleandroperez

jleandroperez Apr 19, 2017

Contributor

Note to self: Okay. Relax. Breathe. It's just lack of alignment.

This comment has been minimized.

@catehstn
@@ -120,7 +122,6 @@ - (BOOL)application:(UIApplication *)application willFinishLaunchingWithOptions:
- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions
{
DDLogVerbose(@"didFinishLaunchingWithOptions state: %d", application.applicationState);
[self.window makeKeyAndVisible];

This comment has been minimized.

@jleandroperez

jleandroperez Apr 19, 2017

Contributor

Out of curiosity: Is this actually not needed? instantiating a UIWindow makes it Key and Visible by default?

This comment has been minimized.

@astralbodies

astralbodies Apr 19, 2017

Member

@jleandroperez We're calling the same method a bit later in this method.

@jleandroperez
Copy link
Contributor

jleandroperez left a comment

This is a masterpiece Aaron!!!

:shipit:

@astralbodies

This comment has been minimized.

Copy link
Member

astralbodies commented Apr 19, 2017

@astralbodies astralbodies merged commit bd1ccbb into develop Apr 19, 2017

1 check passed

Buddybuild : WordPress/57a120bbe0f5520100e11c19 Build succeeded
Details

@astralbodies astralbodies deleted the issue/7046-testing-slim-launch branch Apr 19, 2017

@bummytime

This comment has been minimized.

Copy link
Member

bummytime commented Apr 19, 2017

Nice work @astralbodies 鉂楋笍馃

@nheagy

This comment has been minimized.

Copy link
Contributor

nheagy commented Apr 19, 2017

I'm really excited to see the tests run faster!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment