Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NRE when accessing RxApp.SuspensionHost #2181

Merged
merged 2 commits into from Sep 29, 2019

Conversation

@worldbeater
Copy link
Member

worldbeater commented Sep 29, 2019

What kind of change does this PR introduce?

This PR fixes RxApp.SuspensionHost being null in a unit test runner starting from ReactiveUI 10.3.1. We stumbled upon this when trying to update Avalonia.ReactiveUI to use latest ReactiveUI 10.3.1, there we got 4 failing tests due to RxApp.SuspensionHost containing the null value — a NullReferenceException was thrown from the AutoSuspendHelper constructor.

What is the current behavior?

Starting from ReactiveUI 10.3.1, RxApp.SuspensionHost doesn't get initialized if a unit test runner is detected. This is probably caused by the upgrade to Splat 9 where the unit test mode detector started working, there is no NullReferenceException on ReactiveUI 10.2.2 or lower.

https://github.com/reactiveui/ReactiveUI/blob/master/src/ReactiveUI/RxApp.cs#L110-L119

What is the new behavior?

RxApp.SuspensionHost is now initialized before the ModeDetector check.

What might this PR break?

Nothing. This is required for AvaloniaUI/Avalonia#3045

Don't allow RxApp.SuspensionHost to stay uninitialized.
@worldbeater worldbeater added the bug label Sep 29, 2019
@worldbeater worldbeater marked this pull request as ready for review Sep 29, 2019
@worldbeater worldbeater requested a review from reactiveui/core-team as a code owner Sep 29, 2019
@codecov

This comment has been minimized.

Copy link

codecov bot commented Sep 29, 2019

Codecov Report

Merging #2181 into master will increase coverage by 0.45%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2181      +/-   ##
==========================================
+ Coverage   61.28%   61.74%   +0.45%     
==========================================
  Files         107      107              
  Lines        4063     4062       -1     
  Branches      575      578       +3     
==========================================
+ Hits         2490     2508      +18     
+ Misses       1395     1375      -20     
- Partials      178      179       +1
Impacted Files Coverage Δ
src/ReactiveUI/RxApp.cs 71.64% <50%> (+2.52%) ⬆️
...tiveUI/Bindings/Converter/EqualityTypeConverter.cs 75.55% <0%> (-2.23%) ⬇️
src/ReactiveUI/Suspension/SuspensionHost.cs 50% <0%> (+50%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5b93917...e2e6a29. Read the comment docs.

@glennawatson glennawatson merged commit 89c4621 into master Sep 29, 2019
3 of 4 checks passed
3 of 4 checks passed
codecov/patch 50% of diff hit (target 61.28%)
Details
ReactiveUI-CI Build #10.3.8+56e73d57bb succeeded
Details
codecov/project 61.74% (+0.45%) compared to 5b93917
Details
license/cla All CLA requirements met.
@glennawatson glennawatson deleted the suspension-host-nre branch Sep 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.