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

tests: add a case for invalid context value #2030

Merged
merged 6 commits into from
Nov 4, 2020
Merged

Conversation

snitin315
Copy link
Member

What kind of change does this PR introduce?
tests

Did you add tests for your changes?
yes
If relevant, did you update the documentation?
NA
Summary
add a case for invalid context value

Does this PR introduce a breaking change?
No

Other information
No

@snitin315 snitin315 requested a review from a team as a code owner November 3, 2020 10:18
@snitin315
Copy link
Member Author

fixed for windows.

@alexander-akait
Copy link
Member

/cc @webpack/cli-team

@snitin315 snitin315 requested a review from anshumanv November 4, 2020 13:52
Copy link
Member

@anshumanv anshumanv left a comment

Choose a reason for hiding this comment

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

Let's do invalid values for all core flags?

@snitin315
Copy link
Member Author

snitin315 commented Nov 4, 2020

Mostly we are using general tests for example for optimization-*, output-* flags. We use mapping so we actually don't on which flag we are at the moment.

We have specific test cases only for a few options like devtool, cache, records etc. We can write invalid values tests in those.

@anshumanv
Copy link
Member

That makes sense, let's cover others too then

@alexander-akait
Copy link
Member

Will we do it in another PR?

@snitin315
Copy link
Member Author

Yes, I will check and snatch them all in future PR.

@anshumanv
Copy link
Member

Sure let's merge this one then

@snitin315 snitin315 merged commit 172da46 into master Nov 4, 2020
@snitin315 snitin315 deleted the tests/context branch November 4, 2020 15:06
@snitin315
Copy link
Member Author

Thanks!

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.

4 participants