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

chore: update materialize tests to run mode #6038

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

jakovljevic-mladen
Copy link
Member

Description:
Updated materialize tests to use run mode.

I have removed one of the tests (named should materialize stream never completes) because it was completely the same as the test before, it only used cold instead of hot, but emissions and the subscriptions points were the same.

Related issue (if exists):
None

@jakovljevic-mladen
Copy link
Member Author

The error from the pipelines does not look like it was related to this PR.

At first, I wasn't able to reproduce it locally, but then I saw a change from #6033 where this line was introduced: npm install --no-save typescript@latest tslib@latest @types/node@latest. After I run it and after I run npm run compile, I managed to reproduce the same results locally, but then my IDE started complaining that I should run npm install again since tslib versions do not match.
image
@cartant, maybe you could help here? Is it possible to ignore this error for this PR or is there something that should be done regarding tslib version?

@cartant
Copy link
Collaborator

cartant commented Feb 21, 2021

I'll have a look at it.

... but then my IDE started complaining

You won't want to run exactly what happens in CI locally. As you've noticed, there is a CI task that forcibly installs the latest TS - so that we get notice of any breakage (like the one you've encountered) - and that also means that it needs to install the latest tslib and Node.js types. Forcibly installing the latest in the CI task is no big deal, as everything is thrown away afterwards. If that task is run locally, you'll end up with installed modules that don't match the package.json semver.

@jakovljevic-mladen
Copy link
Member Author

If that task is run locally, you'll end up with installed modules that don't match the package.json semver.

Yeah, I had to reinstall everything from the ground after installing the latest stuff, but by doing so, I just wanted to make sure I can reproduce an error locally.

I'll have a look at it.

Thanks 👍

Copy link
Collaborator

@cartant cartant left a comment

Choose a reason for hiding this comment

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

LGTM. The CI issues should be fixed in #6039.

@cartant cartant merged commit 7208ea7 into ReactiveX:master Feb 22, 2021
@jakovljevic-mladen jakovljevic-mladen deleted the materialize_run_mode branch February 23, 2021 07:25
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

Successfully merging this pull request may close these issues.

None yet

2 participants