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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds a tslint rule for unmocking when doing tests with tracking #1174

Merged
merged 1 commit into from Aug 23, 2018

Conversation

Projects
None yet
4 participants
@orta
Member

orta commented Aug 23, 2018

We've talked about doing these before, but me and @ansor4 wasted a bunch of time on something we can easily make a linter rule for - so this fixes #1172 and now we have at least one custom lint rule, so the next ones are much easier to write.

Making the rules is simple, and I added docs so it's easy for the next person.

akjsbndfak 2018-08-22 20_04_55

馃帀

@orta orta requested review from alloy, damassi, l2succes and zephraph Aug 23, 2018

}
}
module.exports = { Rule: Rule }

This comment has been minimized.

@orta

orta Aug 23, 2018

Member

This has to be a .js file, so this gives you most of the typing ( // @ts-check and the jsdoc ) but none of the ts compilation overhead.

@artsyit

This comment has been minimized.

Contributor

artsyit commented Aug 23, 2018

Deploy preview for artsy-reaction ready!

Built with commit 03d2e2d

https://deploy-preview-1174--artsy-reaction.netlify.com

"You need to un-mock react-tracking if you are using mockTracking in this file."
const fix = new Lint.Fix("fix-tracking", [
new Lint.Replacement(0, 0, `jest.unmock("react-tracking")\n`),
])

This comment has been minimized.

@alloy

alloy Aug 23, 2018

Member

Nice 馃憣

// Does it include mockTracking but not un-mock react-tracking?
if (
sourceFile.text.includes("mockTracking") &&
!sourceFile.text.includes(`jest.unmock("react-tracking")`)

This comment has been minimized.

@alloy

alloy Aug 23, 2018

Member

I鈥檓 pretty sure people at Artsy write single quotes because of ergonomics and let prettier handle the rewriting to use double quotes, meaning that this won鈥檛 catch that until after saving/prettifying. I don鈥檛 think it鈥檒l be an actual issue, but it does mean it will take at least one save before they get to see this.

This comment has been minimized.

@orta

orta Aug 24, 2018

Member

I can live with this, aye

@alloy

alloy approved these changes Aug 23, 2018

Great stuff, both for the specific rule and the hoot example being set 馃憦

@ansor4 ansor4 merged commit eaf8498 into master Aug 23, 2018

7 checks passed

Peril All green. Good on 'ya.
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: relay Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
ci/circleci: type-check Your tests passed on CircleCI!
Details
ci/circleci: update-cache Your tests passed on CircleCI!
Details
deploy/netlify Deploy preview ready!
Details

@damassi damassi deleted the tslibnnt_rule branch Aug 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment