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

throttle tests - some description and their implementation are inconsistent #5360

Closed
BioPhoton opened this issue Mar 21, 2020 · 4 comments
Closed

Comments

@BioPhoton
Copy link
Contributor

Bug Report

Current Behavior
In the specs of throttle described here:
https://github.com/ReactiveX/rxjs/blob/master/spec/operators/throttle-spec.ts#L384
and implemented here:
https://github.com/ReactiveX/rxjs/blob/master/spec/operators/throttle-spec.ts#L411

as well as here:
https://github.com/ReactiveX/rxjs/blob/master/spec/operators/throttle-spec.ts#L385
and implemented here:
https://github.com/ReactiveX/rxjs/blob/master/spec/operators/throttle-spec.ts#L397

It should test against { leading: false, trailing: true } but does test against { leading: true, trailing: true }

If I put false the test fails, which means there is an inconsistency.

Expected behavior
The test should pass and the description and implementation should be consistent.

Possible Solution
I suggest implementing the right behavior in the throttle implementation.

Additional context/Screenshots
throttle-inconsistent-test

@BioPhoton
Copy link
Contributor Author

@cartant if this is a bug I would have the proper fix and could make a PR for the suggested change.

@cartant
Copy link
Collaborator

cartant commented Apr 6, 2020

I doubt it's a bug, as that test's marble sources and expectation are identical to those in the other test - further above it - with same description. If leading is changed to true, the expectation is going to have to change.

It looks to me like the test was copied and not updated.

@BioPhoton
Copy link
Contributor Author

Single values witl leading false trailing true don't pass.
Thought this is what should happen.

@cartant
Copy link
Collaborator

cartant commented Apr 6, 2020

Single values witl leading false trailing true don't pass.

I don't understand. What does this have to do with what you raised in the issue? AFAICT, you linked to two tests with different descriptions that have identical marbles and expectations. If that's the case, the expectations need to be changed as well as the code. If it's not the case, a more thorough explanation is needed, as I don't understand your comment.

cartant added a commit to cartant/rxjs that referenced this issue Apr 30, 2021
@benlesh benlesh closed this as completed in 6d62574 May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants