ExpectThunk proposal #19

obj-p · 2019-02-14T01:57:04Z

Hey all, this is just a thought that I've been exploring previously with ActionCreators and just open for discussion if it has a place somewhere in ReSwift. Here's and example of the previous rendition:
https://github.com/jjgp/ReddiX-iOS/blob/master/UNITests/Actions/ListingsActionsTests.swift

To make Thunks more testable especially when facing async things like mocking requests with MockingJay or OHHTTPStubs I created an ExpectThunk that is a subclass of XCTestExpectation.

If this is to be then there would have to be a subspec or other podspec for consumers to be able to pod install into their tests.

Cheer,

ReSwift-ThunkTests/Tests.swift

ReSwift-ThunkTests/ExpectThunk.swift

ReSwift-ThunkTests/Tests.swift

mjarvis · 2019-02-14T16:58:04Z

Hey, this look great! I can see adding this as a sub-spec to making testing thunks easier.
One improvement I can see making is adding another dispatches function that takes a closure (Action) -> Void to be able to manually match and assert on actions outside of the Equatable restriction (maybe some action is difficult to make equatable).

ReSwift-ThunkTests/ExpectThunk.swift

obj-p · 2019-02-14T19:30:15Z

@mjarvis great feedback! I will incorporate and leave this open a tad longer in case there is any other feedback. Then I will close and make some refactors and reopen the PR.

mjarvis · 2019-02-14T19:31:41Z

@jjgp you are welcome to leave the PR open and make changes in-place. We'll use a squash & merge to merge when its ready so any WIP commits will get squashed out.

ReSwift-ThunkTests/ExpectThunk.swift

mjarvis · 2019-02-20T16:44:05Z

ReSwiftExpectThunk.podspec

@@ -0,0 +1,29 @@
+Pod::Spec.new do |spec|


I think this would probably be better suited as a subspec rather than a separate pod inside the same repo.
See https://guides.cocoapods.org/syntax/podspec.html#subspec and also https://guides.cocoapods.org/syntax/podspec.html#default_subspecs for omitting this from the default.

I would create two subspecs Core, and Expect, then set Core as the default_subspec

I'm all for making it a subspec. I went with this route first to test it in a demo app. I will attempt a subspec next. Will report back on this.

Second thing I would be for adding a blurb in the README.md if that's cool.

I believe I got the subspec down. I have a test project here: https://github.com/jjgp/ExpectThunkDemo

I'd appreciate a sanity check since I did run into CocoaPods fun with a sigabrt at test time, however, I managed to resolve it.

Will work on the blurb in a bit.

sigabrt will show up if the test target is a child target in the Podfile:

target 'ExpectThunkDemo' do # Comment the next line if you're not using Swift and don't want to use dynamic frameworks use_frameworks! # Pods for ExpectThunkDemo pod 'ReSwiftThunk', :git => '[email protected]:jjgp/ReSwift-Thunk.git', :branch => 'expect-thunk-proposal' target 'ExpectThunkDemoTests' do inherit! :search_paths pod 'ReSwiftThunk/ExpectThunk', :git => '[email protected]:jjgp/ReSwift-Thunk.git', :branch => 'expect-thunk-proposal' end end

obj-p · 2019-02-21T16:25:31Z

~~travis is failing and I'll take a look in a bit~~

ReSwift-ThunkTests/ExpectThunk.swift

xavierLowmiller · 2019-04-17T11:21:21Z

Are there any news on this?

I don't mean to be pushy, but we'd love to use this.

Or, since this is 'just' for tests, do you think it's viable to target this branch in our project?

obj-p · 2019-04-17T11:34:45Z

Appreciate the interest! I’ll get back to it today. It should be near complete. After I double check I’ll ping for some reviews. Cheers,

mjarvis · 2019-04-17T15:37:15Z

ReSwiftThunk.podspec

+ end
+
+ spec.subspec "ExpectThunk" do |sp|
+ sp.dependency "ReSwiftThunk/Core"


If you remove this dependency, perhaps the inherited podfile example will work? I think it might be double-importing Core when inherit is included in the test target pod specifications.

Edit: Though, really, cocoapods dependency stuff should really handle this automatically...

Good thought, however, I wasn't able to get it to build this way. I'm attempting with CocoaPods 1.6.1. It may be an open issue with CocoaPods 7195 having child targets with subspecs. I'll follow up there, it does seem splitting the targets is the workaround in the Podfile at the moment.

DivineDominion · 2019-04-18T16:38:39Z

I'm by no means a CocoaPods expert and almost never use it. Is it common to nest test targets inside the library/app target, or just an edge case?

Totally missed this whole PR -- looks very cool!

obj-p · 2019-04-18T19:16:55Z

So CocoaPods defaults to nested targets on a pod init. That being said having them separate is common and often used as a workaround for certain issues. Even documentation such as RxSwift shows them as separate:

# Podfile
use_frameworks!

target 'YOUR_TARGET_NAME' do
    pod 'RxSwift',    '~> 4.0'
    pod 'RxCocoa',    '~> 4.0'
end

# RxTest and RxBlocking make the most sense in the context of unit/integration tests
target 'YOUR_TESTING_TARGET' do
    pod 'RxBlocking', '~> 4.0'
    pod 'RxTest',     '~> 4.0'
end

DivineDominion · 2019-05-17T12:13:19Z

@jjgp Do you need any assistance to make things work? I'd very much like to integrate this rather sooner than later :)

Also, what do you think about exposing a record of dispatched actions or similar, so one can write e.g. 3 dispatches assertions and make sure only 3 actions have been dispatched in total (e.g. there's nothing on top of that).

obj-p · 2019-05-18T15:51:11Z

Appreciate it @DivineDominion! I'm going to get a version of that dispatches in over the weekend. I had a few other thoughts as well.

The only thing I'm unsure about is CocoaPods. I tend to lean that breaking it into separate targets is acceptable for the end user. The issue sited on CocoaPods repo has been moved to a further milestone and perhaps I will have to contribute there.

I'll report back by the end of the weekend.

obj-p · 2019-05-18T15:52:30Z

@DivineDominion currently I do not enforce the order of getStates within the dispatches/getStates chain. I haven't really thought about it, do you have an opinion of it?

obj-p · 2019-05-18T18:39:43Z

Work in progress, I have included the dispatches record as well as built in a way for ExpectThunk to mark parts of the chain that were not reached.

ReSwift-ThunkTests/Tests.swift

ReSwift-ThunkTests/ExpectThunk.swift

obj-p · 2019-05-18T19:34:48Z

Looks like the following Podfile works as well:

platform :ios, '12.1'

target 'ExpectThunkDemo' do
  pod 'ReSwiftThunk', :git => '[email protected]:jjgp/ReSwift-Thunk.git', :branch => 'expect-thunk-proposal'
  target 'ExpectThunkDemoTests' do
    pod 'ReSwiftThunk/ExpectThunk', :git => '[email protected]:jjgp/ReSwift-Thunk.git', :branch => 'expect-thunk-proposal'
  end
end

Moral of the story is that if your using the use_frameworks! option then it will SIGABRT in the tests probably because the framework isn't being dynamically linked or something like that.

Here is a branch where it works with the above Podfile sans use_frameworks!:
https://github.com/jjgp/ExpectThunkDemo/compare/separate-targets

Looking back at this comment, that is the distinction too: #19 (comment)

I'm leaning more towards this is fine CocoaPods wise as the user has a few options and the framework linking seems to make sense.

obj-p · 2019-05-18T23:42:32Z

updated failure descriptions:

DivineDominion · 2019-05-20T09:53:21Z

Moral of the story is that if your using the use_frameworks! option then it will SIGABRT in the tests probably because the framework isn't being dynamically linked or something like that.

Do I read this correctly: this makes it impossible to use use_Frameworks! for devs who use this sub-spec of the pod?

obj-p · 2019-05-20T10:31:05Z

use_frameworks!may still be used as long as the testing target is defined outside of the source target in the Podfile like in this demo project
https://github.com/jjgp/ExpectThunkDemo/blob/master/Podfile

It seems if the test target is nested in the source target and it declares use_frameworks! then the issue presents itself.

It could be related to this issue with CocoaPods as listed here: CocoaPods/CocoaPods#7195
~~Although I don’t see others using use_frameworks there.~~ Actually: CocoaPods/CocoaPods#7195 (comment)

Another way I’ve seen test helpers implemented is as a separate CocoaPod entirely such as RxTest. I would have to test, however, I’m inclined to think it wouldn’t have the issue seen here.

DivineDominion · 2019-05-20T12:19:19Z

Cool! I've tested the code in my app and really like the test helper. So if someone with more CocoaPods experience (maybe @mjarvis or @danielmartinprieto) accept the CocoaPods side of things, I think we should merge this 👍

DivineDominion · 2019-06-05T10:01:43Z

@mjarvis @danielmartinprieto Does any one of you have an opinion regarding the pod subspec mentioned above?

mjarvis · 2019-06-05T15:25:24Z

I'm okay with a bit of awkwardness as long as:

It doesn't affect installation of the main project
Its well documented

obj-p · 2019-06-05T17:26:53Z

I will update the README to make it more clear. Also looking to the future I’ll look into Swift Package Manager and it may make for a cleaner integration!

obj-p · 2019-06-11T13:53:20Z

Just updated the README to make it explicit to not nest the targets if using the subspec 😄

DivineDominion · 2019-06-11T19:40:13Z

Great 👍

Could you add your addition to the top of the changelog? (Sorry I'm serving all this info piecemeal and asking you to always edit one more thing :))

obj-p · 2019-06-13T17:48:10Z

Just added a blurb to the changlog. I hope I put it in the expected place 😄

DivineDominion · 2019-06-13T19:06:22Z

Just noticed you enabled editing by maintainers, that's good :) I edited in some details. Thanks for all this!

ExpectThunk proposal

04b9cff

houndci-bot reviewed Feb 14, 2019

View reviewed changes

obj-p commented Feb 14, 2019

View reviewed changes

ReSwift-ThunkTests/Tests.swift Outdated Show resolved Hide resolved

wowza trailing spaces

dc21ace

obj-p added 2 commits February 14, 2019 14:19

adding ability to pass custom assertion block

6d06241

refactoring ExpectThunk away from subclassing, and various tidbits

0118c5e

houndci-bot reviewed Feb 14, 2019

View reviewed changes

ReSwift-ThunkTests/ExpectThunk.swift Outdated Show resolved Hide resolved

ReSwift-ThunkTests/ExpectThunk.swift Outdated Show resolved Hide resolved

obj-p added 3 commits February 19, 2019 10:16

using createThunkMiddleware in ExpectThunk

0bf5468

renaming ExpectThunk members

fc20af6

adding boundary guard and some cleanup

aa3e1ce

houndci-bot reviewed Feb 20, 2019

View reviewed changes

ReSwift-ThunkTests/ExpectThunk.swift Outdated Show resolved Hide resolved

obj-p added 6 commits February 19, 2019 20:07

code style changes

a674e51

trailing whitespace

6b86566

removing type as it's inferred

0b543e4

adding podspec

5e154b5

adding public accessors to getsState and run

7e638f1

updating podspec

754dbce

mjarvis reviewed Feb 20, 2019

View reviewed changes

removing podspec and adding subspec

063b055

obj-p commented Feb 21, 2019

View reviewed changes

ReSwift-ThunkTests/ExpectThunk.swift Outdated Show resolved Hide resolved

obj-p added 3 commits February 21, 2019 12:22

adding preprocessor flag

c313633

using isEmpty for fun

4d8c279

adding a blurb on expect-thunk in README.md

2440ec2

removing equatable from FakeAction

a671f34

mjarvis reviewed Apr 17, 2019

View reviewed changes

obj-p mentioned this pull request Apr 17, 2019

Can't use subspec in child target whose parent target uses parent spec CocoaPods/CocoaPods#7195

Open

1 task

houndci-bot reviewed May 18, 2019

View reviewed changes

New wait/run API as well as dispatched record

0922abe

adding wait failed

8618792

updating failure descriptions

30f9a96

adding blurb about ExpectThunk subspec

ff7234f

Adding CHANGELOG comment for ExpectThunk

5bf1d69

add PR reference

f272dc9

DivineDominion approved these changes Jun 13, 2019

View reviewed changes

DivineDominion merged commit f266ebf into ReSwift:master Jun 13, 2019

obj-p deleted the expect-thunk-proposal branch June 21, 2019 01:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ExpectThunk proposal #19

ExpectThunk proposal #19

obj-p commented Feb 14, 2019

mjarvis commented Feb 14, 2019

obj-p commented Feb 14, 2019

mjarvis commented Feb 14, 2019

mjarvis Feb 20, 2019

obj-p Feb 20, 2019

mjarvis Feb 20, 2019

obj-p Feb 21, 2019

obj-p Feb 21, 2019

obj-p commented Feb 21, 2019 •

edited

Loading

xavierLowmiller commented Apr 17, 2019

obj-p commented Apr 17, 2019

mjarvis Apr 17, 2019 •

edited

Loading

obj-p Apr 17, 2019

DivineDominion commented Apr 18, 2019 •

edited

Loading

obj-p commented Apr 18, 2019

DivineDominion commented May 17, 2019 •

edited

Loading

obj-p commented May 18, 2019

obj-p commented May 18, 2019

obj-p commented May 18, 2019

obj-p commented May 18, 2019 •

edited

Loading

obj-p commented May 18, 2019

DivineDominion commented May 20, 2019

obj-p commented May 20, 2019 •

edited

Loading

DivineDominion commented May 20, 2019

DivineDominion commented Jun 5, 2019

mjarvis commented Jun 5, 2019

obj-p commented Jun 5, 2019

obj-p commented Jun 11, 2019 •

edited

Loading

DivineDominion commented Jun 11, 2019

obj-p commented Jun 13, 2019

DivineDominion commented Jun 13, 2019

ExpectThunk proposal #19

ExpectThunk proposal #19

Conversation

obj-p commented Feb 14, 2019

mjarvis commented Feb 14, 2019

obj-p commented Feb 14, 2019

mjarvis commented Feb 14, 2019

mjarvis Feb 20, 2019

Choose a reason for hiding this comment

obj-p Feb 20, 2019

Choose a reason for hiding this comment

mjarvis Feb 20, 2019

Choose a reason for hiding this comment

obj-p Feb 21, 2019

Choose a reason for hiding this comment

obj-p Feb 21, 2019

Choose a reason for hiding this comment

obj-p commented Feb 21, 2019 • edited Loading

xavierLowmiller commented Apr 17, 2019

obj-p commented Apr 17, 2019

mjarvis Apr 17, 2019 • edited Loading

Choose a reason for hiding this comment

obj-p Apr 17, 2019

Choose a reason for hiding this comment

DivineDominion commented Apr 18, 2019 • edited Loading

obj-p commented Apr 18, 2019

DivineDominion commented May 17, 2019 • edited Loading

obj-p commented May 18, 2019

obj-p commented May 18, 2019

obj-p commented May 18, 2019

obj-p commented May 18, 2019 • edited Loading

obj-p commented May 18, 2019

DivineDominion commented May 20, 2019

obj-p commented May 20, 2019 • edited Loading

DivineDominion commented May 20, 2019

DivineDominion commented Jun 5, 2019

mjarvis commented Jun 5, 2019

obj-p commented Jun 5, 2019

obj-p commented Jun 11, 2019 • edited Loading

DivineDominion commented Jun 11, 2019

obj-p commented Jun 13, 2019

DivineDominion commented Jun 13, 2019

obj-p commented Feb 21, 2019 •

edited

Loading

mjarvis Apr 17, 2019 •

edited

Loading

DivineDominion commented Apr 18, 2019 •

edited

Loading

DivineDominion commented May 17, 2019 •

edited

Loading

obj-p commented May 18, 2019 •

edited

Loading

obj-p commented May 20, 2019 •

edited

Loading

obj-p commented Jun 11, 2019 •

edited

Loading