Skip to content

Commit

Permalink
Feature/handle multithreaded notifications (#1051)
Browse files Browse the repository at this point in the history 
* Be more careful about threading and notifications

Make sure that Nimble doesn't try to access notification properties from the main
thread as they may have been created on another thread and get upset
(most likely when Core Data is involved).
Fixes #973

* Fix typo in CONTRIBUTING.md file

* Update Tests/NimbleTests/Helpers/BackgroundThreadObject.swift

Co-authored-by: Rachel Brindle <you@subluminal.net>

* Fix tests for PostNotificationTest

---------

Co-authored-by: John McKerrell <john@mckerrell.net>
  • Loading branch information
@younata @johnmckerrell
younata and johnmckerrell committed Apr 11, 2023
1 parent d605306 commit 605d2bd
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 2 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ swift test
by name.
- Make sure your pull request includes any necessary updates to the
README or other documentation.
- Be sure the unit tests for both the OS X and iOS targets of Nimble
- Be sure the unit tests for both the OS X and iOS targets of Nimble pass
before submitting your pull request. You can run all the OS X & iOS unit
tests using `./test`.
- If you've added a file to the project, make sure it's included in both
Expand Down
10 changes: 10 additions & 0 deletions Nimble.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,9 @@
472FD13B1B9E0CFE00C7B8DA /* HaveCountTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 472FD1361B9E094B00C7B8DA /* HaveCountTest.swift */; };
4793854D1BA0BB2500296F85 /* ObjCHaveCountTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 4793854C1BA0BB2500296F85 /* ObjCHaveCountTest.m */; };
4793854E1BA0BB2500296F85 /* ObjCHaveCountTest.m in Sources */ = {isa = PBXBuildFile; fileRef = 4793854C1BA0BB2500296F85 /* ObjCHaveCountTest.m */; };
52F5CD6527EE571C00B19809 /* BackgroundThreadObject.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52F5CD6427EE571C00B19809 /* BackgroundThreadObject.swift */; };
52F5CD6627EE571C00B19809 /* BackgroundThreadObject.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52F5CD6427EE571C00B19809 /* BackgroundThreadObject.swift */; };
52F5CD6727EE571C00B19809 /* BackgroundThreadObject.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52F5CD6427EE571C00B19809 /* BackgroundThreadObject.swift */; };
62FB326223B78BF90047BED9 /* BeginWithPrefix.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62FB326123B78BF90047BED9 /* BeginWithPrefix.swift */; };
62FB326323B78BF90047BED9 /* BeginWithPrefix.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62FB326123B78BF90047BED9 /* BeginWithPrefix.swift */; };
62FB326423B78BF90047BED9 /* BeginWithPrefix.swift in Sources */ = {isa = PBXBuildFile; fileRef = 62FB326123B78BF90047BED9 /* BeginWithPrefix.swift */; };
Expand Down Expand Up @@ -320,6 +323,7 @@
857D184F2536124400D8693A /* BeWithinTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 857D184D2536123F00D8693A /* BeWithinTest.swift */; };
857D18502536124400D8693A /* BeWithinTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 857D184D2536123F00D8693A /* BeWithinTest.swift */; };
857D18512536124500D8693A /* BeWithinTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 857D184D2536123F00D8693A /* BeWithinTest.swift */; };
8913626D29E5C2F500AD535E /* BackgroundThreadObject.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52F5CD6427EE571C00B19809 /* BackgroundThreadObject.swift */; };
892FDF1329D3EA7700523A80 /* AsyncExpression.swift in Sources */ = {isa = PBXBuildFile; fileRef = 892FDF1229D3EA7700523A80 /* AsyncExpression.swift */; };
892FDF1429D3EA7700523A80 /* AsyncExpression.swift in Sources */ = {isa = PBXBuildFile; fileRef = 892FDF1229D3EA7700523A80 /* AsyncExpression.swift */; };
892FDF1529D3EA7700523A80 /* AsyncExpression.swift in Sources */ = {isa = PBXBuildFile; fileRef = 892FDF1229D3EA7700523A80 /* AsyncExpression.swift */; };
Expand Down Expand Up @@ -759,6 +763,7 @@
472FD1341B9E085700C7B8DA /* HaveCount.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HaveCount.swift; sourceTree = "<group>"; };
472FD1361B9E094B00C7B8DA /* HaveCountTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = HaveCountTest.swift; sourceTree = "<group>"; };
4793854C1BA0BB2500296F85 /* ObjCHaveCountTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ObjCHaveCountTest.m; sourceTree = "<group>"; };
52F5CD6427EE571C00B19809 /* BackgroundThreadObject.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackgroundThreadObject.swift; sourceTree = "<group>"; };
62FB326123B78BF90047BED9 /* BeginWithPrefix.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BeginWithPrefix.swift; sourceTree = "<group>"; };
62FB326523B78D4A0047BED9 /* BeginWithPrefixTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BeginWithPrefixTest.swift; sourceTree = "<group>"; };
6CAEDD091CAEA86F003F1584 /* LinuxSupport.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LinuxSupport.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -890,6 +895,7 @@
1F14FB63194180C5009F2A08 /* utils.swift */,
1F0648CB19639F5A001F9C46 /* ObjectWithLazyProperty.swift */,
898F28AF25D9F4C30052B8D0 /* AlwaysFailMatcher.swift */,
52F5CD6427EE571C00B19809 /* BackgroundThreadObject.swift */,
);
path = Helpers;
sourceTree = "<group>";
Expand Down Expand Up @@ -1724,6 +1730,7 @@
7B13BA111DD361EB00C9098C /* ObjCContainElementSatisfyingTest.m in Sources */,
1F925EF6195C147800ED456B /* BeCloseToTest.swift in Sources */,
106112C62251E13B000A5848 /* BeResultTest.swift in Sources */,
52F5CD6627EE571C00B19809 /* BackgroundThreadObject.swift in Sources */,
1F4A56791A3B32E3009E1637 /* ObjCBeGreaterThanOrEqualToTest.m in Sources */,
A8A3B6F6207329DD00E25A08 /* SatisfyAllOfTest.swift in Sources */,
AE7ADE491C80C00D00B94CD3 /* MatchErrorTest.swift in Sources */,
Expand Down Expand Up @@ -1874,6 +1881,7 @@
89F5E09D290C37B8001F9377 /* OnFailureThrowsTest.swift in Sources */,
1F5DF1A71BDCA10200C3A531 /* EqualTest.swift in Sources */,
106112C72251E13B000A5848 /* BeResultTest.swift in Sources */,
52F5CD6727EE571C00B19809 /* BackgroundThreadObject.swift in Sources */,
CD79C9AA1D2CC848004B6F9A /* ObjCBeLessThanOrEqualToTest.m in Sources */,
1F5DF1931BDCA10200C3A531 /* SynchronousTest.swift in Sources */,
CD79C9A11D2CC83B004B6F9A /* ObjCBeCloseToTest.m in Sources */,
Expand Down Expand Up @@ -2030,6 +2038,7 @@
7B13BA101DD361EA00C9098C /* ObjCContainElementSatisfyingTest.m in Sources */,
1F925EF7195C147800ED456B /* BeCloseToTest.swift in Sources */,
1F4A567A1A3B32E3009E1637 /* ObjCBeGreaterThanOrEqualToTest.m in Sources */,
52F5CD6527EE571C00B19809 /* BackgroundThreadObject.swift in Sources */,
A8A3B6F5207329DC00E25A08 /* SatisfyAllOfTest.swift in Sources */,
AE7ADE4A1C80C00D00B94CD3 /* MatchErrorTest.swift in Sources */,
1F4A568C1A3B3407009E1637 /* ObjCBeTrueTest.m in Sources */,
Expand Down Expand Up @@ -2135,6 +2144,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
8913626D29E5C2F500AD535E /* BackgroundThreadObject.swift in Sources */,
D95F8945267EA1E8004B1B4D /* BeNilTest.swift in Sources */,
D95F8949267EA1E8004B1B4D /* MatchTest.swift in Sources */,
D95F8951267EA1EE004B1B4D /* utils.swift in Sources */,
Expand Down
5 changes: 4 additions & 1 deletion Sources/Nimble/Matchers/PostNotification.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ import Foundation

internal class NotificationCollector {
private(set) var observedNotifications: [Notification]
private(set) var observedNotificationDescriptions: [String]
private let notificationCenter: NotificationCenter
private let names: Set<Notification.Name>
private var tokens: [NSObjectProtocol]

required init(notificationCenter: NotificationCenter, names: Set<Notification.Name> = []) {
self.notificationCenter = notificationCenter
self.observedNotifications = []
self.observedNotificationDescriptions = []
self.names = names
self.tokens = []
}
Expand All @@ -21,6 +23,7 @@ internal class NotificationCollector {
return notificationCenter.addObserver(forName: name, object: nil, queue: nil) { [weak self] notification in
// linux-swift gets confused by .append(n)
self?.observedNotifications.append(notification)
self?.observedNotificationDescriptions.append(stringify(notification))
}
}

Expand Down Expand Up @@ -71,7 +74,7 @@ private func _postNotifications<Out>(
if collector.observedNotifications.isEmpty {
actualValue = "no notifications"
} else {
actualValue = "<\(stringify(collector.observedNotifications))>"
actualValue = "<\(stringify(collector.observedNotificationDescriptions))>"
}

var result = try predicate.satisfies(collectorNotificationsExpression)
Expand Down
14 changes: 14 additions & 0 deletions Tests/NimbleTests/Helpers/BackgroundThreadObject.swift
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import Foundation
import Nimble

// Simulates an object that *really* cares what thread it is run on.
// Supposed to replicate what happens if a NSManagedObject ends up
// in a notification and is picked up by Nimble
class BackgroundThreadObject: CustomDebugStringConvertible {
var debugDescription: String {
if Thread.isMainThread {
fail("This notification was accessed on the main thread when it should have been handled on the thread it was received on")
}
return "BackgroundThreadObject"
}
}
40 changes: 40 additions & 0 deletions Tests/NimbleTests/Matchers/PostNotificationTest.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,46 @@ final class PostNotificationTest: XCTestCase {
}
}.toEventually(postNotifications(equal([testNotification]), from: notificationCenter))
}

func testFailsWhenNotificationIsPostedUnexpectedly() {
let n1 = Notification(name: Notification.Name("Foo"), object: nil)
failsWithErrorMessage("expected to not equal <[\(n1)]>, got <[\(n1)]>") {
expect {
self.notificationCenter.post(n1)
}.toNot(postNotifications(equal([n1]), from: self.notificationCenter))
}
}

func testPassesWhenNotificationIsNotPosted() {
let n1 = Notification(name: Notification.Name("Foo"), object: nil)
let n2 = Notification(name: Notification.Name(n1.name.rawValue + "a"), object: nil)
expect {
self.notificationCenter.post(n2)
}.toNever(postNotifications(equal([n1]), from: self.notificationCenter))
}

func testPassesWhenNotificationIsPostedFromADifferentThread() {
let n1 = Notification(name: Notification.Name("Foo"), object: nil)
expect {
OperationQueue().addOperations([BlockOperation {
let backgroundThreadObject = BackgroundThreadObject()
let n2 = Notification(name: Notification.Name("Bar"), object: backgroundThreadObject)
self.notificationCenter.post(n2)
}], waitUntilFinished: true)
self.notificationCenter.post(n1)
}.to(postNotifications(contain([n1]), from: notificationCenter))
}

func testPassesWhenNotificationIsPostedFromADifferentThreadAndToNotCalled() {
let n1 = Notification(name: Notification.Name("Foo"), object: nil)
expect {
OperationQueue().addOperations([BlockOperation {
let backgroundThreadObject = BackgroundThreadObject()
let n2 = Notification(name: Notification.Name(n1.name.rawValue + "a"), object: backgroundThreadObject)
self.notificationCenter.post(n2)
}], waitUntilFinished: true)
}.toNot(postNotifications(equal([n1]), from: notificationCenter))
}

#if os(macOS)
func testPassesWhenAllExpectedNotificationsarePostedInDistributedNotificationCenter() {
Expand Down

0 comments on commit 605d2bd

Please sign in to comment.