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

3.x: Introduce property rx3.scheduler.use-nanotime #7169

Merged
merged 1 commit into from Jan 28, 2021
Merged

3.x: Introduce property rx3.scheduler.use-nanotime #7169

merged 1 commit into from Jan 28, 2021

Conversation

SergejIsbrecht
Copy link
Contributor

@SergejIsbrecht SergejIsbrecht commented Jan 27, 2021

Fixes #7154

@SergejIsbrecht
Copy link
Contributor Author

@akarnokd ,

I got a problem, when I try to test the code, because when the class is loaded, one can not change set variable:

`private static final String DRIFT_USE_NANOTIME = "rx3.scheduler.drift-use-nanotime";

Resolution:

  • no tests
  • define a own test set, which tests the nanoTime use-case and configure this test-src-set to run each class in a new JVM
  • find a Junit-runner which loads the test-classes via a own ClassLoader
  • remove "final" for DRIFT_USE_NANOTIME and set it for testing, which might not be desirable, because it can be changed later.

What are your thoughts on this issue regarding testing?

@akarnokd akarnokd added this to the 3.1 milestone Jan 27, 2021
@codecov
Copy link

codecov bot commented Jan 27, 2021

Codecov Report

Merging #7169 (6cfd483) into 3.x (a99b2e0) will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##                3.x    #7169      +/-   ##
============================================
+ Coverage     99.50%   99.56%   +0.05%     
- Complexity     6739     6742       +3     
============================================
  Files           747      747              
  Lines         47324    47328       +4     
  Branches       6360     6360              
============================================
+ Hits          47091    47120      +29     
+ Misses          108       94      -14     
+ Partials        125      114      -11     
Impacted Files Coverage Δ Complexity Δ
...va/io/reactivex/rxjava3/schedulers/Schedulers.java 100.00% <ø> (ø) 12.00 <0.00> (ø)
...main/java/io/reactivex/rxjava3/core/Scheduler.java 100.00% <100.00%> (ø) 16.00 <4.00> (+2.00)
...nternal/operators/parallel/ParallelReduceFull.java 93.06% <0.00%> (-1.99%) 2.00% <0.00%> (ø%)
...ternal/operators/flowable/FlowableWindowTimed.java 97.19% <0.00%> (-0.29%) 5.00% <0.00%> (ø%)
...3/internal/operators/flowable/FlowablePublish.java 99.00% <0.00%> (ø) 17.00% <0.00%> (-1.00%)
...ternal/operators/observable/ObservableFlatMap.java 97.87% <0.00%> (+0.35%) 3.00% <0.00%> (ø%)
...nternal/operators/observable/ObservableReplay.java 100.00% <0.00%> (+0.53%) 22.00% <0.00%> (ø%)
...ternal/operators/completable/CompletableMerge.java 98.64% <0.00%> (+1.35%) 2.00% <0.00%> (ø%)
...internal/operators/flowable/FlowableSwitchMap.java 100.00% <0.00%> (+1.41%) 3.00% <0.00%> (ø%)
.../operators/observable/ObservableFlatMapSingle.java 96.82% <0.00%> (+1.58%) 2.00% <0.00%> (ø%)
... and 5 more

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 a99b2e0...6cfd483. Read the comment docs.

Copy link
Member

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this affects now(), let's call the property just rx3.scheduler.use-nanotime.

@SergejIsbrecht SergejIsbrecht changed the title [WIP] 3.x: Introduce property rx3.scheduler.drift-use-nanotime 3.x: Introduce property rx3.scheduler.drift-use-nanotime Jan 28, 2021
@akarnokd akarnokd changed the title 3.x: Introduce property rx3.scheduler.drift-use-nanotime 3.x: Introduce property rx3.scheduler.use-nanotime Jan 28, 2021
@SergejIsbrecht
Copy link
Contributor Author

SergejIsbrecht commented Jan 28, 2021

@akarnokd , I'll changed the commit message as-well, to match the new property name.

@akarnokd
Copy link
Member

No need for that.

@akarnokd akarnokd merged commit 171c846 into ReactiveX:3.x Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.x Feature: introduce abstract time source for scheduler impl
3 participants