You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I went on an investigation on a code-base I have been working on (unfortunately closed-source, so I am unable to share more details). Beyond assert.isTrue not working (listed in #38) I found some other issues.
assert.isEmpty and assert.isFalse were also broken in a verified way. My assumption was any assertions on assert that don't take arguments may be broken, but I haven't verified.
When removing dirty-chai from our tests, I also found instances where sinon-chai assertions and assertions following the chai-as-promised eventually method were silently not working. These could have been due to plugin order or version mismatches. We were loading chai-as-promised after dirty-chai (against the README). According to #32, that might not be the issue, so I wanted to bring it up in case there is an actually underlying issue besides that.
All in all, I was able to confirm that due to these issues we had a significant number of tests that were silently broken and an even larger number that could have silently broken.
I looked into fixing these issues myself, but ended up removing dirty-chai. Partially because I didn't see an easy way to make a fix that wouldn't have edge cases. We are also eventually moving away from chai long-term, so it made sense to simplify our setup so future maintainers wouldn't have to be aware of how dirty-chai works in addition to chai itself.
Since these issues have gone unfixed for a few years I am assuming they aren't trivial to fix. I am wondering if it might be worthwhile to call some of these issues out more prominently in the README in the meantime. That way potential adopters could at least be aware of some of the limitations.
Definitely appreciate the work on the library and understand the frustration with the property accessor assertions. In my opinion, this library makes the tests look much better and I wasn't happy to roll it back from a stylistic perspective 😄
The text was updated successfully, but these errors were encountered:
lelandmiller
changed the title
Silent failures on multiple assertions
Multiple assertions not working
Mar 9, 2022
I went on an investigation on a code-base I have been working on (unfortunately closed-source, so I am unable to share more details). Beyond assert.isTrue not working (listed in #38) I found some other issues.
assert.isEmpty
andassert.isFalse
were also broken in a verified way. My assumption was any assertions on assert that don't take arguments may be broken, but I haven't verified.When removing dirty-chai from our tests, I also found instances where sinon-chai assertions and assertions following the chai-as-promised
eventually
method were silently not working. These could have been due to plugin order or version mismatches. We were loading chai-as-promised after dirty-chai (against the README). According to #32, that might not be the issue, so I wanted to bring it up in case there is an actually underlying issue besides that.All in all, I was able to confirm that due to these issues we had a significant number of tests that were silently broken and an even larger number that could have silently broken.
I looked into fixing these issues myself, but ended up removing dirty-chai. Partially because I didn't see an easy way to make a fix that wouldn't have edge cases. We are also eventually moving away from chai long-term, so it made sense to simplify our setup so future maintainers wouldn't have to be aware of how dirty-chai works in addition to chai itself.
Since these issues have gone unfixed for a few years I am assuming they aren't trivial to fix. I am wondering if it might be worthwhile to call some of these issues out more prominently in the README in the meantime. That way potential adopters could at least be aware of some of the limitations.
Definitely appreciate the work on the library and understand the frustration with the property accessor assertions. In my opinion, this library makes the tests look much better and I wasn't happy to roll it back from a stylistic perspective 😄
The text was updated successfully, but these errors were encountered: