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

Allow running in node environment #64

Merged
merged 1 commit into from
Oct 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const jestConfig = require('kcd-scripts/jest')

module.exports = Object.assign(jestConfig, {
testEnvironment: 'jest-environment-jsdom',
testEnvironment: 'jest-environment-node',
testURL: 'http://localhost/',
setupTestFrameworkScriptFile: '<rootDir>/setupTests.js',
})
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
"build": "kcd-scripts build",
"lint": "kcd-scripts lint",
"test": "kcd-scripts test",
"test:all": "npm test && npm test -- --env jsdom",
"test:update": "npm test -- --updateSnapshot --coverage",
"validate": "kcd-scripts validate",
"validate": "kcd-scripts validate build,lint,test:all",
"setup": "npm install && npm run validate -s",
"precommit": "kcd-scripts precommit"
},
Expand All @@ -40,6 +41,7 @@
"redent": "^2.0.0"
},
"devDependencies": {
"jsdom": "^12.2.0",
"kcd-scripts": "^0.44.0"
},
"eslintConfig": {
Expand Down
8 changes: 8 additions & 0 deletions src/__tests__/helpers/document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
if (global.document) {
module.exports = global.document
} else {
const {JSDOM} = require('jsdom')
const {window} = new JSDOM()

module.exports = window.document
}
2 changes: 2 additions & 0 deletions src/__tests__/helpers/test-utils.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import document from './document'

function render(html) {
const container = document.createElement('div')
container.innerHTML = html
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/to-be-in-the-document.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import document from './helpers/document'

test('.toBeInTheDocument', () => {
document.body.innerHTML = `
<span data-testid="html-element"><span>Html Element</span></span>
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/to-have-style.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {render} from './helpers/test-utils'
import document from './helpers/document'

describe('.toHaveStyle', () => {
test('handles positive test cases', () => {
Expand Down
29 changes: 1 addition & 28 deletions src/__tests__/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
import {checkDocumentKey, deprecate} from '../utils'

function matcherMock() {}
import {deprecate} from '../utils'

test('deprecate', () => {
const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
Expand All @@ -16,28 +14,3 @@ test('deprecate', () => {

spy.mockRestore()
})

test('checkDocumentKey', () => {
const fakeKey = 'fakeKey'
const realKey = 'documentElement'
const badKeyMessage = `${fakeKey} is undefined on document but is required to use ${
matcherMock.name
}.`

const badDocumentMessage = `document is undefined on global but is required to use ${
matcherMock.name
}.`

expect(() =>
checkDocumentKey(document, realKey, matcherMock),
).not.toThrowError()

expect(() => {
checkDocumentKey(document, fakeKey, matcherMock)
}).toThrow(badKeyMessage)

expect(() => {
//eslint-disable-next-line no-undef
checkDocumentKey(undefined, realKey, matcherMock)
}).toThrow(badDocumentMessage)
})
15 changes: 7 additions & 8 deletions src/to-be-in-the-document.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@ import {
stringify,
RECEIVED_COLOR as receivedColor,
} from 'jest-matcher-utils'
import {checkHtmlElement, checkDocumentKey} from './utils'
import {checkHtmlElement} from './utils'

export function toBeInTheDocument(element) {
if (element !== null) {
checkHtmlElement(element, toBeInTheDocument, this)
}

checkDocumentKey(global.document, 'documentElement', toBeInTheDocument)
const pass =
element === null ? false : element.ownerDocument.contains(element)

return {
pass: document.documentElement.contains(element),
pass,
message: () => {
return [
matcherHint(
Expand All @@ -22,11 +23,9 @@ export function toBeInTheDocument(element) {
'',
),
'',
receivedColor(`${stringify(
document.documentElement.cloneNode(false),
)} ${this.isNot ? 'contains:' : 'does not contain:'} ${stringify(
element ? element.cloneNode(false) : element,
)}
receivedColor(`${stringify(element.ownerDocument.cloneNode(false))} ${
this.isNot ? 'contains:' : 'does not contain:'
} ${stringify(element ? element.cloneNode(false) : element)}
`),
].join('\n')
},
Expand Down
2 changes: 2 additions & 0 deletions src/to-be-visible.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import {matcherHint, printReceived} from 'jest-matcher-utils'
import {checkHtmlElement} from './utils'

function isStyleVisible(element) {
const {getComputedStyle} = element.ownerDocument.defaultView

const {display, visibility, opacity} = getComputedStyle(element)
return (
display !== 'none' &&
Expand Down
5 changes: 3 additions & 2 deletions src/to-contain-html.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {matcherHint, printReceived} from 'jest-matcher-utils'
import {checkHtmlElement} from './utils'

function checkHtmlText(htmlText, ...args) {
function checkHtmlText(element, htmlText, ...args) {
const DOMParser = element.ownerDocument.defaultView.DOMParser
const htmlElement =
typeof htmlText === 'string'
? new DOMParser().parseFromString(htmlText, 'text/html').body.firstChild
Expand All @@ -11,7 +12,7 @@ function checkHtmlText(htmlText, ...args) {

export function toContainHTML(container, htmlText) {
checkHtmlElement(container, toContainHTML, this)
checkHtmlText(htmlText, toContainHTML, this)
checkHtmlText(container, htmlText, toContainHTML, this)

return {
pass: container.outerHTML.includes(htmlText),
Expand Down
4 changes: 2 additions & 2 deletions src/to-have-focus.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ export function toHaveFocus(element) {
checkHtmlElement(element, toHaveFocus, this)

return {
pass: document.activeElement === element,
pass: element.ownerDocument.activeElement === element,
message: () => {
return [
matcherHint(`${this.isNot ? '.not' : ''}.toHaveFocus`, 'element', ''),
'',
'Received:',
` ${printReceived(document.activeElement)}`,
` ${printReceived(element.ownerDocument.activeElement)}`,
].join('\n')
},
}
Expand Down
5 changes: 3 additions & 2 deletions src/to-have-style.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import jestDiff from 'jest-diff'
import chalk from 'chalk'
import {checkHtmlElement, checkValidCSS} from './utils'

function getStyleDeclaration(css) {
function getStyleDeclaration(document, css) {
const copy = document.createElement('div')
copy.style = css
const styles = copy.style
Expand Down Expand Up @@ -48,8 +48,9 @@ function expectedDiff(expected, computedStyles) {
export function toHaveStyle(htmlElement, css) {
checkHtmlElement(htmlElement, toHaveStyle, this)
checkValidCSS(css, toHaveStyle, this)
const {getComputedStyle} = htmlElement.ownerDocument.defaultView

const expected = getStyleDeclaration(css)
const expected = getStyleDeclaration(htmlElement.ownerDocument, css)
const received = getComputedStyle(htmlElement)

return {
Expand Down
39 changes: 3 additions & 36 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ class HtmlElementTypeError extends Error {

function checkHtmlElement(htmlElement, ...args) {
if (
!(htmlElement instanceof HTMLElement) &&
!(htmlElement instanceof SVGElement)
!htmlElement.ownerDocument &&
!(htmlElement instanceof htmlElement.ownerDocument.HTMLElement) &&
!(htmlElement instanceof htmlElement.ownerDocument.SVGElement)
Copy link
Member

@gnapse gnapse Oct 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this condition be this instead?

  if (
    !htmlElement.ownerDocument ||
    (
      !(htmlElement instanceof htmlElement.ownerDocument.HTMLElement) &&
      !(htmlElement instanceof htmlElement.ownerDocument.SVGElement)
    )
  ) {
    throw new HtmlElementTypeError(htmlElement, matcherFn, context, validTypes)
  }

If there's no htmlElement.ownerDocument, it makes no sense to go and evaluate the rest of the condition, because the rest of the conditional depends entirely on htmlElement.ownerDocument not being undefined.

Please, let me know if I'm correct or not so far. If I am, I have more things to say, but I wanted to confirm up to this point at least

cc @sheerun

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, here it is the additional part of what I wanted to say.

I think the above is never surfaced because checkHtmlElement is coincidentally always called with a valid html element that has an ownerDocument.

So we never really get to the point where the instanceof parts of the conditions are evaluated even. The thing is, when I fixed the condition locally, I get that htmlElement.ownerDocument.HTMLElement is undefined. The same with SVGElement.

This is the point where I'm no longer sure how to continue this fix. Care to chime in @sheerun?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More insight into this. Code coverage is not at 100% for this src/utils.js file. However, I'm not sure how that's not being captured by the travis build, which has always flagged code coverage less than 100%.

😕

) {
throw new HtmlElementTypeError(htmlElement, ...args)
}
Expand Down Expand Up @@ -74,39 +75,6 @@ function checkValidCSS(css, ...args) {
}
}

class InvalidDocumentError extends Error {
constructor(message, matcherFn) {
super()

/* istanbul ignore next */
if (Error.captureStackTrace) {
Error.captureStackTrace(this, matcherFn)
}

this.message = message
}
}

function checkDocumentKey(document, key, matcherFn) {
if (typeof document === 'undefined') {
throw new InvalidDocumentError(
`document is undefined on global but is required to use ${
matcherFn.name
}.`,
matcherFn,
)
}

if (typeof document[key] === 'undefined') {
throw new InvalidDocumentError(
`${key} is undefined on document but is required to use ${
matcherFn.name
}.`,
matcherFn,
)
}
}

function display(value) {
return typeof value === 'string' ? value : stringify(value)
}
Expand Down Expand Up @@ -147,7 +115,6 @@ function normalize(text) {
}

export {
checkDocumentKey,
checkHtmlElement,
checkValidCSS,
deprecate,
Expand Down