Fix missing Cancel() in tests that don't consume the entire source #32
Conversation
|
cc @Silv3rcircl3 |
|
@akarnokd could you please add yourself to the copyright file? Thx. Will merge once it's merged on the jvm too. |
|
Done in #33. |
|
Ping @ktoso, thoughts? :) |
| @@ -0,0 +1,192 @@ | |||
| using NUnit.Framework; | |||
There was a problem hiding this comment.
should rs.net also have copy header as we did in the jvm sources?
(general remark, all sources should get it)
|
Will have a look - was in my todo list, fell asleep in the evening though, kind of tired ;) |
| { | ||
| b.Append("\r\n-------------------------------"); | ||
|
|
||
| b.Append("\r\nat ").Append(stacks[t.Key]); |
There was a problem hiding this comment.
hm, so Append(stacks: collection), but that won't insert the \r\nat before each of the entries right? So has to be done via normal loop I guess, unless this API has something magical to it?
There was a problem hiding this comment.
No and doesn't matter. Unlike Java, the console output is not pretty printed and stacktraces are not appear as links in C#.
There was a problem hiding this comment.
If no one is looking at it why print it at all...? Without even a space or newlines between elements?
There was a problem hiding this comment.
It printed the methods that needed attention when I was fixing the underlying problem. Plus it will print problems in case new TCK tests are added in the future.
Some of the TCK tests didn't cancel their
ISubscriptions when they stopped consuming the source which could lead to resource leaks.This is also an issue in the JVM version of the tck: reactive-streams/reactive-streams-jvm#346