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

Some tests improvements #110

Merged
merged 4 commits into from
Feb 13, 2022
Merged

Some tests improvements #110

merged 4 commits into from
Feb 13, 2022

Conversation

vadz
Copy link
Contributor

@vadz vadz commented Feb 13, 2022

To avoid having any more problems like the one from #105, I've changed the mock function in the tests return empty string rather than undefined when the option is not defined, to be consistent with the real function and added a new test checking that everything works without any options being set.

I've also made 2 other cosmetic changes that could be [not] applied independently of the 2 other ones to make test output nicer to read.

Sorry again for the breakage!

This is more consistent with the behaviour of the actual function, which
always returns a string, and would have avoided introducing an error in
dc9bdec (Fix validation of tmake-server-xxx options, 2022-02-12)
In addition to checking for the options validation, also check that not
having any options and using the defaults works too.
Remove the extraneous "be" from recurrent "should be handle" typo.

No real changes.
This avoids cluttering the tests output with these messages, each of
which is given once per test.
Copy link
Collaborator

@dscho dscho left a comment

Choose a reason for hiding this comment

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

Nice!

@dscho dscho merged commit dd46ba0 into mxschmitt:master Feb 13, 2022
@dscho
Copy link
Collaborator

dscho commented Feb 13, 2022

@mxschmitt FWIW I will not tag a new release because there are not really any user-visible changes in this here PR, just a lot of future-proofing, which I like.

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