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

Redundant tests for the first/last operators #3717

Closed
cartant opened this issue May 21, 2018 · 3 comments
Closed

Redundant tests for the first/last operators #3717

cartant opened this issue May 21, 2018 · 3 comments
Assignees

Comments

@cartant
Copy link
Collaborator

cartant commented May 21, 2018

Whilst fixing the typing problems in the test suite, I found a seemingly redundant test for the first operator:

it('should support type guards without breaking previous behavior', () => {

first does not accept a used-defined type guard, so the test does nothing. Indeed, it will fail once type checking is enabled for the test suite.

Whether or not the test should be removed or the signature of first should be modified to accept a user-defined type guard, I don't know. Hence the opening of this issue.

cartant added a commit to cartant/rxjs that referenced this issue May 21, 2018
@cartant
Copy link
Collaborator Author

cartant commented May 21, 2018

There is a similar problem with last.

@cartant cartant changed the title Redundant test for first Redundant tests for the first/last operators May 21, 2018
cartant added a commit to cartant/rxjs that referenced this issue May 21, 2018
@benlesh
Copy link
Member

benlesh commented May 21, 2018

Yeah, it just looks poorly written. We do need to make sure that type guards work with operators like first, last, filter, etc.

cartant added a commit to cartant/rxjs that referenced this issue May 21, 2018
@cartant
Copy link
Collaborator Author

cartant commented May 21, 2018

I'll come back and fix these up, then, once the test suite is type-checked. Some overload signatures will need to be added for the type guards to effect narrowed observable types.

benlesh pushed a commit that referenced this issue May 22, 2018
* chore(test): check e* operators

* chore(test): fix every types

* chore(test): check f* operators

* chore(test): fix filter types

* chore(test): fix find types

* chore(test): fix findIndex types

* chore(test): comment out failing test for first

See #3717

* chore(test): check g* operators

* chore(test): fix groupBy types

* chore(test): prepare upcoming checks

To facilitate automatic merge conflict resolution.
cartant added a commit to cartant/rxjs that referenced this issue May 22, 2018
cartant added a commit to cartant/rxjs that referenced this issue May 24, 2018
benlesh pushed a commit that referenced this issue May 25, 2018
* chore(test): check i*, l* operators

* chore(test): comment out failing test for last

See #3717

* chore(test): check m* operators

* chore(test): fix map types

* chore(test): fix max types

* chore(test): fix min types

* chore(test): check o* operators

* chore(test): fix observeOn types
@lock lock bot locked as resolved and limited conversation to collaborators Aug 12, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants