-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add toHaveAttribute custom matcher (closes #43) #44
Changes from 4 commits
0618715
bfeb26a
12db312
aaf03ae
c191008
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import extensions from './jest-extensions' | ||
|
||
const {toBeInTheDOM, toHaveTextContent} = extensions | ||
expect.extend({toBeInTheDOM, toHaveTextContent}) | ||
const {toBeInTheDOM, toHaveTextContent, toHaveAttribute} = extensions | ||
expect.extend({toBeInTheDOM, toHaveTextContent, toHaveAttribute}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,12 @@ | ||
import {matcherHint, printReceived, printExpected} from 'jest-matcher-utils' //eslint-disable-line import/no-extraneous-dependencies | ||
//eslint-disable-next-line import/no-extraneous-dependencies | ||
import { | ||
matcherHint, | ||
printReceived, | ||
printExpected, | ||
stringify, | ||
RECEIVED_COLOR as receivedColor, | ||
EXPECTED_COLOR as expectedColor, | ||
} from 'jest-matcher-utils' | ||
import {matches} from './utils' | ||
|
||
function getDisplayName(subject) { | ||
|
@@ -9,10 +17,30 @@ function getDisplayName(subject) { | |
} | ||
} | ||
|
||
function checkHtmlElement(htmlElement) { | ||
if (!(htmlElement instanceof HTMLElement)) { | ||
throw new Error( | ||
`The given subject is a ${getDisplayName( | ||
htmlElement, | ||
)}, not an HTMLElement`, | ||
) | ||
} | ||
} | ||
|
||
const assertMessage = (assertionName, message, received, expected) => | ||
`${matcherHint(`${assertionName}`, 'received', '')} \n${message}: ` + | ||
`${printExpected(expected)} \nReceived: ${printReceived(received)}` | ||
|
||
function printAttribute(name, value) { | ||
return value === undefined ? name : `${name}=${stringify(value)}` | ||
} | ||
|
||
function getAttributeComment(name, value) { | ||
return value === undefined | ||
? `element.hasAttribute(${stringify(name)})` | ||
: `element.getAttribute(${stringify(name)}) === ${stringify(value)}` | ||
} | ||
|
||
const extensions = { | ||
toBeInTheDOM(received) { | ||
getDisplayName(received) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just noticed that this actually isn't doing anything. I'm pretty sure we should assign it a value called There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I noticed that too, but I preferred no to touch it here as it's unrelated. I do have a few code cleanup proposals for this file, that I was meaning to provide in a separate PR. For instance, the pattern of returning something different on the matchers depending on wether they Also it'd be good to bring the other two matchers messages up to the standards that you have proposed for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I'd love it if you could clean this file up a bit in another PR! Let's just see what we need to do to increase coverage back to 100%. Open |
||
|
@@ -42,13 +70,7 @@ const extensions = { | |
}, | ||
|
||
toHaveTextContent(htmlElement, checkWith) { | ||
if (!(htmlElement instanceof HTMLElement)) | ||
throw new Error( | ||
`The given subject is a ${getDisplayName( | ||
htmlElement, | ||
)}, not an HTMLElement`, | ||
) | ||
|
||
checkHtmlElement(htmlElement) | ||
const textContent = htmlElement.textContent | ||
const pass = matches(textContent, htmlElement, checkWith) | ||
if (pass) { | ||
|
@@ -75,6 +97,41 @@ const extensions = { | |
} | ||
} | ||
}, | ||
|
||
toHaveAttribute(htmlElement, name, expectedValue) { | ||
checkHtmlElement(htmlElement) | ||
const isExpectedValuePresent = expectedValue !== undefined | ||
const hasAttribute = htmlElement.hasAttribute(name) | ||
const receivedValue = htmlElement.getAttribute(name) | ||
return { | ||
pass: isExpectedValuePresent | ||
? hasAttribute && receivedValue === expectedValue | ||
: hasAttribute, | ||
message: () => { | ||
const to = this.isNot ? 'not to' : 'to' | ||
const receivedAttribute = receivedColor( | ||
hasAttribute | ||
? printAttribute(name, receivedValue) | ||
: 'attribute was not found', | ||
) | ||
const expectedMsg = `Expected the element ${to} have attribute:\n ${expectedColor( | ||
printAttribute(name, expectedValue), | ||
)}` | ||
const matcher = matcherHint( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @antoaravinth Not in this case. The messages I wanted to generate do not comply with the predefined format that BTW, not even the two already existing custom matchers used this function. It's only being used in one of them. I do not think we can expect all or even most matchers to always adhere to this expected/received constrained message format. I was checking at jest's own codebase and how they approach generating messages, and the most common functionality they have across all them is encoded in all the functions they also publish in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I got it. Just wanted to check. Cool, thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That being said, although I do not object myself to how the code ended up in this function for generating the message, I get it if it looks too complex, and I'm of course open to any suggestions to improve it or even the resulting messages as well. I would not like though to do so by sacrificing too much of the friendliness of the resulting messages. This matcher, although simple in the actual condition being tested, covers a few cases (e.g. it can test if the attribute is merely present or not, or it can test also that it has a specific value). Therefore the messages for the positive and negative cases, each of these handling the two cases of the attr value being tested or not, yields four different kind of resulting messages. |
||
`${this.isNot ? '.not' : ''}.toHaveAttribute`, | ||
'element', | ||
printExpected(name), | ||
{ | ||
secondArgument: isExpectedValuePresent | ||
? printExpected(expectedValue) | ||
: undefined, | ||
comment: getAttributeComment(name, expectedValue), | ||
}, | ||
) | ||
return `${matcher}\n\n${expectedMsg}\nReceived:\n ${receivedAttribute}` | ||
}, | ||
} | ||
}, | ||
} | ||
|
||
export default extensions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering here, can't we use
printReceived
? Is thatRECEIVED_COLOR
does something apart fromprintReceived
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
printReceived
usesRECEIVED_COLOR
internally, but it also stringifies the argument. But in the new code I added I wanted to showattribute-name="value"
all in the received or expected color, without stringifying all of it. If I usedprintReceived
/printExpected
I'd have the wholeattribute-name="value"
itself surrounded by double quotes (and possibly those inner quotes escaped then).