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

assertASTsAreEqual: don't depend on Chai #78

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lazywithclass
Copy link

Addresses the need not to depend on chai in assertASTsAreEqual, as per #35.

While pondering the changes I've made I gave a look at the coverage for assertASTsAreEqual and found out that there are two branches not covered, while being not particularly fond of 100% coverage per se I still think it gives some useful insights.

First unconvered branch is the right side of this expression, I could not find a suitable test input that could lead there, my hunch is that it's related to this any: I believe that if we could express all types involved then the type checker could do the heavy lifting for us looking for pattern matching exhaustiveness.

Second one is this , and it too, I believe, is related to the above any.

I've looked around and I've seen that value of type FeatureDescription is being used as

At this point I would ask a colleague for guidance, showing my findings, to discuss if and how to progress this.


General notes

  • chaijs' expect when expecting exception checks if the expected message is part of the message in the exception, this might be confusing sometimes, my take would be to use the whole exception message
  • in my experience Jest considers it.only and describe.only only if focusing on the file they're used (because tests run in parallel); if you're interested I could submit a PR that allows to filter tests, unless I'm missing something crucial in the development process

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.

1 participant