Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Fix configuration tests when comparing rules #4346

Merged
merged 1 commit into from
Dec 6, 2018

Conversation

mogzol
Copy link
Contributor

@mogzol mogzol commented Dec 6, 2018

PR checklist

Overview of change:

Several of the tests in configurationTests.ts make comparisons on config rules using assert.deepEqual, which doesn't work on Maps. Because of this, configs that differed could still pass the tests. This commit fixes that by calling demap on the maps before the comparisons.

Is there anything you'd like reviewers to focus on?

This is strictly fixing the tests. One quirk I noticed is boolean configurations result in [] as the rule arguments, while normal configurations result in undefined. If this behavior is not desired, it should probably be fixed in a separate PR.

Several of the tests in configurationTests.ts make comparisons on
config rules using assert.deepEqual, which doesn't work on Maps.
Because of this, configs that differed could still pass the tests.
This commit fixes that by calling demap on the maps before the
comparisons.
@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint, @mogzol! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@JoshuaKGoldberg
Copy link
Contributor

Nice catch, thanks for this @mogzol!

If this behavior is not desired, it should probably be fixed in a separate PR.

Yeah the difference seems odd, likely unintended. Would you mind filing an issue for that please? Seems like an interesting quirk to discuss.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants