{"action":"created","issue":{"url":"https://api.github.com/repos/chaijs/chai/issues/870","repository_url":"https://api.github.com/repos/chaijs/chai","labels_url":"https://api.github.com/repos/chaijs/chai/issues/870/labels{/name}","comments_url":"https://api.github.com/repos/chaijs/chai/issues/870/comments","events_url":"https://api.github.com/repos/chaijs/chai/issues/870/events","html_url":"https://github.com/chaijs/chai/issues/870","id":189854412,"number":870,"title":"chai.assert.strictEqual should warn when comparing two undefined values","user":{"login":"mlee","id":10884538,"avatar_url":"https://avatars.githubusercontent.com/u/10884538?v=3","gravatar_id":"","url":"https://api.github.com/users/mlee","html_url":"https://github.com/mlee","followers_url":"https://api.github.com/users/mlee/followers","following_url":"https://api.github.com/users/mlee/following{/other_user}","gists_url":"https://api.github.com/users/mlee/gists{/gist_id}","starred_url":"https://api.github.com/users/mlee/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mlee/subscriptions","organizations_url":"https://api.github.com/users/mlee/orgs","repos_url":"https://api.github.com/users/mlee/repos","events_url":"https://api.github.com/users/mlee/events{/privacy}","received_events_url":"https://api.github.com/users/mlee/received_events","type":"User","site_admin":false},"labels":[{"id":151444991,"url":"https://api.github.com/repos/chaijs/chai/labels/more-discussion-needed","name":"more-discussion-needed","color":"bfd4f2","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":10,"created_at":"2016-11-16T21:39:16Z","updated_at":"2016-11-18T13:53:51Z","closed_at":null,"body":"Current behavior:\r\n```\r\nchai.assert.strictEqual(undefined, undefined) // -> passes\r\n```\r\n\r\nDesired behavior:\r\n```\r\nchai.assert.strictEqual(undefined, undefined) // -> passes with warning\r\n```\r\n\r\nIf asserting on two undefined values, it is better to just use `chai.assert.isUndefined`. The problem with the current behavior is if the expected and actual arguments to `strictEqual` are `undefined` by mistake, the assertion will still pass.\r\n\r\nA (contrived) example:\r\n```\r\nfunction testedFunc() {\r\n  'asdf'; // desired behavior is actually to return 'asdf'\r\n}\r\n\r\n...\r\n\r\nvar expectedObj = {\r\n  thing: 'asdf'\r\n};\r\n\r\nassert.strictEqual(testedFunc(), expectedObj.thin); // second argument should have been expectedObj.thing, but this assertion still passes\r\n```\r\n"},"comment":{"url":"https://api.github.com/repos/chaijs/chai/issues/comments/261536543","html_url":"https://github.com/chaijs/chai/issues/870#issuecomment-261536543","issue_url":"https://api.github.com/repos/chaijs/chai/issues/870","id":261536543,"user":{"login":"meeber","id":17260989,"avatar_url":"https://avatars.githubusercontent.com/u/17260989?v=3","gravatar_id":"","url":"https://api.github.com/users/meeber","html_url":"https://github.com/meeber","followers_url":"https://api.github.com/users/meeber/followers","following_url":"https://api.github.com/users/meeber/following{/other_user}","gists_url":"https://api.github.com/users/meeber/gists{/gist_id}","starred_url":"https://api.github.com/users/meeber/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/meeber/subscriptions","organizations_url":"https://api.github.com/users/meeber/orgs","repos_url":"https://api.github.com/users/meeber/repos","events_url":"https://api.github.com/users/meeber/events{/privacy}","received_events_url":"https://api.github.com/users/meeber/received_events","type":"User","site_admin":false},"created_at":"2016-11-18T13:53:51Z","updated_at":"2016-11-18T13:53:51Z","body":"@keithamus Thank you for taking the time to have this discussion! I totally agree that there's a broader issue here that can only be solved through education and writing better tests. The three points of advice that you provided are all spot-on.\r\n\r\nI also agree that there's no way Chai could defend against errors in logic like the one you used in your example; the only solution in such cases is to write better tests. I also agree there are a lot of similarities between my original example, which could be defended against by Chai, and your example, which could not. And I understand the argument that Chai shouldn't defend against the problem in my original example if it can't also defend against the broader problem, such as the one demonstrated in your example, especially because there's a cost associated with fixing the problem in my original example, and because there are other potential solutions such as linting that don't have that cost.\r\n\r\nNevertheless, I'm still on the fence on this particular issue. I feel like it's at least possible that the benefit of protecting against the accidental `undefined` problem demonstrated in my original example is worth the cost of requiring developers to use `isUndefined` instead of equality assertions when testing for `undefined`, even though it's a subclass of a broader problem that can't be completely fixed by Chai. There's a somewhat subjective cost/benefit analysis involved with this that I haven't yet fully resolved. \r\n\r\nThe point is, I'm still not yet convinced either way. I'd like to think about it some more, and I'd also love to hear some more opinions: @vieiralucas @lucasfcosta @shvaikalesh."}}
{"actor":{"display_login":"meeber"}}