Test: Properly assert duplicate key warnings

README.md

@@ -113,6 +113,10 @@ I created Airstream because I found existing solutions were not suitable for bui
 
 Please run `sbt +test` and `sbt scalafmtAll` locally before submitting the PR.
 
+Note that existing tests print this compiler warning in Scala 3:
+- [E029] Pattern Match Exhaustivity Warning: /Users/raquo/code/scala/airstream/src/test/scala-3/com/raquo/airstream/split/SplitMatchOneSpec.scala
+
+This is expected. Ideally I would assert that this warning exists instead of printing it, but I don't think that's possible. I don't want to hide such warnings wholesale, but suggestions for improvement are welcome.
 
 ## Documentation
 
@@ -1612,7 +1616,7 @@ Aside from `splitEither`, we also have `splitOption`, `splitTry`, `splitStatus`,
 
 The `split` operator does not tolerate items with non-unique keys – this is simply invalid input for it, and it will crash and burn if provided such bad data.
 
-Therefore, Airstream offers duplicate key warnings by default. **Your code will still break** if the `split` operator encounters duplicate keys, but Airstream will first print a warning in the browser console listing the duplicate keys at fault.
+Therefore, Airstream offers duplicate key warnings by default. **Your code will still break** if the `split` operator encounters duplicate keys, but Airstream will first report the error as [unhandled](#unhandled-errors-do-not-terminate-the-program) to Airstream, which by default prints it as an error in the browser console, listing the duplicate keys at fault.
 
 Thus, these new warnings do not affect the execution of your code, and can be safely turned on for debugging or turned off for performance. You can adjust this setting both for your entire application, and for individual usages of `split`:
 
@@ -2325,7 +2329,7 @@ Remember, this is just for observers. We have a better way for observables, read
 
 ##### Unhandled Errors Do Not Terminate The Program
 
-In Airstream, unhandled errors do not result in the program terminating. By default they are reported to the console. You can specify different or additional handlers such as `AirstreamError.debuggerErrorCallback` or even a custom handler that effectively terminates your program.
+In Airstream, unhandled errors do not result in the program terminating. By default, they are reported to the console. You can specify different or additional handlers such as `AirstreamError.debuggerErrorCallback` or even a custom handler that effectively terminates your program.
 
 Regardless of this seeming leniency, you should still handle all of your errors at some point before they become _unhandled_. In a good Airstream codebase every _unhandled_ error must be treated as a bug.
 

src/main/scala/com/raquo/airstream/split/DuplicateKeysConfig.scala

@@ -10,16 +10,15 @@ package com.raquo.airstream.split
   *
   * When warnings are enabled, YOUR CODE WILL STILL BREAK if the
   * .split() operator encounters duplicate keys, but it will
-  * first print a warning in the browser console listing the
-  * duplicate keys at fault.
+  * first report an error as `unhandled` in Airstream, which by
+  * default will print it as an error in the browser console,
+  * listing the duplicate keys at fault.
   *
   * We enable this setting by default to aid in debugging. As the
   * end user, you might want to disable this either globally or
   * for specific .split() usages to improve performance on very
   * large lists.
   *
-  * #TODO[Docs]: Add a short section to the docs about this.
-  *
   * #TODO: Add more granular control later, if there is demand for that.
   *  For example, we could instruct Airstream to skip duplicate keys, or
   *  to raise an exception if a duplicate happens. In the latter case

src/test/scala-3/com/raquo/airstream/split/SplitMatchSeqSpec.scala

@@ -1,7 +1,7 @@
 package com.raquo.airstream.split
 
 import com.raquo.airstream.UnitSpec
-import com.raquo.airstream.core.{Observer, Signal, Transaction}
+import com.raquo.airstream.core.{AirstreamError, Observer, Signal, Transaction}
 import com.raquo.airstream.eventbus.EventBus
 import com.raquo.airstream.fixtures.{Effect, TestableOwner}
 import com.raquo.airstream.ownership.{DynamicOwner, DynamicSubscription, ManualOwner, Subscription}
@@ -45,10 +45,27 @@ class SplitMatchSeqSpec extends UnitSpec with BeforeAndAfter {
     override def toString: String = s"Element($id, fooSignal)"
   }
 
+  private val errorEffects = mutable.Buffer[Effect[Throwable]]()
+
+  private val errorCallback = (err: Throwable) => {
+    errorEffects += Effect("unhandled", err)
+    ()
+  }
+
   private val originalDuplicateKeysConfig = DuplicateKeysConfig.default
 
+  before {
+    errorEffects.clear()
+    AirstreamError.registerUnhandledErrorCallback(errorCallback)
+    AirstreamError.unregisterUnhandledErrorCallback(AirstreamError.consoleErrorCallback)
+  }
+
   after {
     DuplicateKeysConfig.setDefault(originalDuplicateKeysConfig)
+
+    AirstreamError.registerUnhandledErrorCallback(AirstreamError.consoleErrorCallback)
+    AirstreamError.unregisterUnhandledErrorCallback(errorCallback)
+    assert(errorEffects.isEmpty) // #Note this fails the test rather inelegantly
   }
 
   def withOrWithoutDuplicateKeyWarnings(code: => Assertion): Assertion = {
@@ -695,17 +712,24 @@ class SplitMatchSeqSpec extends UnitSpec with BeforeAndAfter {
 
     myVar.writer.onNext(FooC("object", 1) :: FooC("object", 1) :: FooC("int_", 1) :: FooO :: FooE.FooE2 :: FooO :: Nil)
 
-    // This should warn, but not throw
-
-    // #TODO[Test] we aren't actually testing that this is logging to the console.
-    //  I'm not sure how to do this without over-complicating things.
-    //  The console warning is printed into the test output, we can at least see it there if / when we look
+    // This should send errors into `unhandled`, but not throw
 
     DuplicateKeysConfig.setDefault(DuplicateKeysConfig.warnings)
 
     myVar.writer.onNext(FooC("object", 1) :: FooC("int_", 1) :: FooO :: FooE.FooE2 :: Nil)
 
     myVar.writer.onNext(FooC("object", 1) :: FooC("object", 1) :: FooC("int_", 1) :: FooO :: FooE.FooE2 :: FooO :: Nil)
+
+    // Foo0 is the duplicate. Key is (matchCaseIndex, Foo0.id)
+    assert(errorEffects.exists { eff =>
+      (
+        eff.name == "unhandled"
+          && eff.value.getMessage.contains("Duplicate keys detected")
+          && eff.value.getMessage.contains(": `(1,object)`, `(2,object)`.")
+        )
+    })
+    errorEffects.size shouldBe 1
+    errorEffects.clear()
   }
 
   it("split list / vector / set / js.array / immutable.seq / collection.seq / option compiles") {

src/test/scala/com/raquo/airstream/misc/SplitSignalSpec.scala

@@ -1,7 +1,7 @@
 package com.raquo.airstream.misc
 
 import com.raquo.airstream.UnitSpec
-import com.raquo.airstream.core.{Observer, Signal, Transaction}
+import com.raquo.airstream.core.{AirstreamError, Observer, Signal, Transaction}
 import com.raquo.airstream.eventbus.EventBus
 import com.raquo.airstream.fixtures.{Effect, TestableOwner}
 import com.raquo.airstream.ownership.{DynamicOwner, DynamicSubscription, ManualOwner, Subscription}
@@ -26,10 +26,27 @@ class SplitSignalSpec extends UnitSpec with BeforeAndAfter {
     override def toString: String = s"Element($id, fooSignal)"
   }
 
+  private val errorEffects = mutable.Buffer[Effect[Throwable]]()
+
+  private val errorCallback = (err: Throwable) => {
+    errorEffects += Effect("unhandled", err)
+    ()
+  }
+
   private val originalDuplicateKeysConfig = DuplicateKeysConfig.default
 
+  before {
+    errorEffects.clear()
+    AirstreamError.registerUnhandledErrorCallback(errorCallback)
+    AirstreamError.unregisterUnhandledErrorCallback(AirstreamError.consoleErrorCallback)
+  }
+
   after {
     DuplicateKeysConfig.setDefault(originalDuplicateKeysConfig)
+
+    AirstreamError.registerUnhandledErrorCallback(AirstreamError.consoleErrorCallback)
+    AirstreamError.unregisterUnhandledErrorCallback(errorCallback)
+    assert(errorEffects.isEmpty) // #Note this fails the test rather inelegantly
   }
 
   def withOrWithoutDuplicateKeyWarnings(code: => Assertion): Assertion = {
@@ -1146,13 +1163,19 @@ class SplitSignalSpec extends UnitSpec with BeforeAndAfter {
 
     // This should warn, but not throw
 
-    // #TODO[Test] we aren't actually testing that this is logging to the console.
-    //  I'm not sure how to do this without over-complicating things.
-    //  The console warning is printed into the test output, we can at least see it there if / when we look
-
     DuplicateKeysConfig.setDefault(DuplicateKeysConfig.warnings)
 
     myVar.writer.onNext(Foo("c", 1) :: Foo("a", 1) :: Foo("b", 1) :: Foo("a", 2) :: Foo("b", 3) :: Nil)
+
+    assert(errorEffects.exists { eff =>
+      (
+        eff.name == "unhandled"
+        && eff.value.getMessage.contains("Duplicate keys detected")
+        && eff.value.getMessage.contains(": `a`, `b`.")
+      )
+    })
+    errorEffects.size shouldBe 1
+    errorEffects.clear()
   }
 
   it("split list / vector / set / js.array / immutable.seq / collection.seq / option compiles") {