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

No logs about empty state for tests with mocked store #2420

Closed
wants to merge 50 commits into from

Conversation

evgenyfedorenko
Copy link
Contributor

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

This code adds an internal setting and a setter for mock store environment.
When set it suppresses the logs about the state which does not exist.

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

Console warn logs numerous logs for mocked store which should not happen

Closes #2363

What is the new behavior?

Logs will be suppressed

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Please take a look at this draft. I made some assumptions here which may not be true. Please comment.

@ngrxbot
Copy link
Collaborator

ngrxbot commented Mar 2, 2020

Preview docs changes for 5c9bd12 at https://previews.ngrx.io/pr2420-5c9bd12/

@alex-okrushko
Copy link
Member

@evgenyfedorenko thanks. Tests would be nice as well 😁

Copy link
Member

@alex-okrushko alex-okrushko left a comment

Choose a reason for hiding this comment

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

Please add tests

@evgenyfedorenko
Copy link
Contributor Author

@evgenyfedorenko thanks. Tests would be nice as well 😁

Was hoping this one will go unnoticed haha, jk.

@@ -14,6 +14,7 @@ export {
export { createAction, props, union } from './action_creator';
export { Store, select } from './store';
export { combineReducers, compose, createReducerFactory } from './utils';
export { setNgrxMockEnvironment, isNgrxMockEnvironment } from './flags';
Copy link
Member

Choose a reason for hiding this comment

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

I suppose not, but is there a way to make the setNgrxMockEnvironment private?
This, to prevent the usage of it to disable the warning.

@timdeschryver timdeschryver added the WIP Not ready for review label Mar 9, 2020
AdditionAddict and others added 14 commits March 11, 2020 07:22
These checks are enabled by default in NgRx 9
* fix(component): add docs overview

closes ngrx#2442

* Update projects/ngrx.io/content/guide/component/index.md

Co-Authored-By: Tim Deschryver <[email protected]>

Co-authored-by: Tim Deschryver <[email protected]>
@brandonroberts
Copy link
Member

@evgenyfedorenko What's the status on this? The PR needs to be rebased on master, and we still need some tests

@evgenyfedorenko
Copy link
Contributor Author

Will try to take a look this week.

@evgenyfedorenko
Copy link
Contributor Author

Added few tests.
@timdeschryver you asked to make this API private. It needs to be available in testing module as well as in store. How else I can share and not make it part of the public API ?

@timdeschryver
Copy link
Member

@evgenyfedorenko I don't think it will be possible, it was an open question to ask if someone knew a way to do this.

@brandonroberts
Copy link
Member

brandonroberts commented May 5, 2020

Something happened to the commits with the merge

@evgenyfedorenko
Copy link
Contributor Author

Not sure what happened, should I recreate my changes on a new branch checked out from master?

@timdeschryver
Copy link
Member

@evgenyfedorenko I think it should resolve itself if you rebase from master, but feel free to create a new branch if that's faster.

@evgenyfedorenko
Copy link
Contributor Author

here is another PR with clean merge #2513

@evgenyfedorenko evgenyfedorenko deleted the mockstore-logsless branch May 6, 2020 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Not ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MockStore selectors resulting in warning logs