Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Commit

Permalink
make usage of jest.mock for react-dom conform to defined behavior
Browse files Browse the repository at this point in the history
This helps prepare for using Babel to transpile TypeScript but is good practice regardless.

- In `code_intelligence.test.tsx`, the `jest.mock('react-dom', ...)` was rejected with a fatal error by Babel (in babel-jest) because it referred to an out-of-scope variable (for why it rejects this, see jestjs/jest#2567). Also, it was undefined behavior that the `jest.mock` of `react-dom` applied to other modules (see "...mocked only for the file that calls `jest.mock`..." at https://jestjs.io/docs/en/jest-object#jestmockmodulename-factory-options). So, this commit passes the `react-dom` `render` function as an argument to the functions under test to make mocking easier and fully explicit without the need for test harness magic.
- Removes unnecessary mocking of `react-dom` in `text_fields.test.tsx`. The mock was never checked.
  • Loading branch information
sqs committed May 12, 2019
1 parent d59816f commit bf1c24d
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 19 deletions.
15 changes: 8 additions & 7 deletions browser/src/libs/code_intelligence/code_intelligence.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
const RENDER = jest.fn()
jest.mock('react-dom', () => ({
createPortal: jest.fn(el => el),
render: RENDER,
unmountComponentAtNode: jest.fn(),
}))

import { Range } from '@sourcegraph/extension-api-classes'
import { uniqueId } from 'lodash'
import renderer from 'react-test-renderer'
Expand All @@ -23,6 +16,8 @@ import { MutationRecordLike } from '../../shared/util/dom'
import { createGlobalDebugMount, createOverlayMount, FileInfo, handleCodeHost } from './code_intelligence'
import { toCodeViewResolver } from './code_views'

const RENDER = jest.fn()

const elementRenderedAtMount = (mount: Element): renderer.ReactTestRendererJSON | undefined => {
const call = RENDER.mock.calls.find(call => call[1] === mount)
return call && call[0]
Expand Down Expand Up @@ -176,6 +171,7 @@ describe('code_intelligence', () => {
platformContext: createMockPlatformContext(),
sourcegraphURL: DEFAULT_SOURCEGRAPH_URL,
telemetryService: NOOP_TELEMETRY_SERVICE,
render: RENDER,
})
)
const overlayMount = document.body.querySelector('.hover-overlay-mount')
Expand All @@ -202,6 +198,7 @@ describe('code_intelligence', () => {
platformContext: createMockPlatformContext(),
sourcegraphURL: DEFAULT_SOURCEGRAPH_URL,
telemetryService: NOOP_TELEMETRY_SERVICE,
render: RENDER,
})
)
const renderedCommandPalette = elementRenderedAtMount(commandPaletteMount)
Expand All @@ -223,6 +220,7 @@ describe('code_intelligence', () => {
platformContext: createMockPlatformContext(),
sourcegraphURL: DEFAULT_SOURCEGRAPH_URL,
telemetryService: NOOP_TELEMETRY_SERVICE,
render: RENDER,
})
)
const globalDebugMount = document.body.querySelector('.global-debug')
Expand Down Expand Up @@ -265,6 +263,7 @@ describe('code_intelligence', () => {
platformContext: createMockPlatformContext(),
sourcegraphURL: DEFAULT_SOURCEGRAPH_URL,
telemetryService: NOOP_TELEMETRY_SERVICE,
render: RENDER,
})
)
const editors = await from(services.editor.editors)
Expand Down Expand Up @@ -328,6 +327,7 @@ describe('code_intelligence', () => {
platformContext: createMockPlatformContext(),
sourcegraphURL: DEFAULT_SOURCEGRAPH_URL,
telemetryService: NOOP_TELEMETRY_SERVICE,
render: RENDER,
})
)
const activeEditor = await from(extensionAPI.app.activeWindowChanges)
Expand Down Expand Up @@ -415,6 +415,7 @@ describe('code_intelligence', () => {
platformContext: createMockPlatformContext(),
sourcegraphURL: DEFAULT_SOURCEGRAPH_URL,
telemetryService: NOOP_TELEMETRY_SERVICE,
render: RENDER,
})
)
let editors = await from(services.editor.editors)
Expand Down
19 changes: 15 additions & 4 deletions browser/src/libs/code_intelligence/code_intelligence.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import * as H from 'history'
import { uniqBy } from 'lodash'
import * as React from 'react'
import { render } from 'react-dom'
import { render as reactDOMRender } from 'react-dom'
import { animationFrameScheduler, EMPTY, from, Observable, of, Subject, Subscription, Unsubscribable } from 'rxjs'
import {
catchError,
Expand Down Expand Up @@ -252,8 +252,11 @@ export function initCodeIntelligence({
codeHost,
platformContext,
extensionsController,
render,
telemetryService,
}: Pick<CodeIntelligenceProps, 'codeHost' | 'platformContext' | 'extensionsController' | 'telemetryService'>): {
}: Pick<CodeIntelligenceProps, 'codeHost' | 'platformContext' | 'extensionsController' | 'telemetryService'> & {
render: typeof reactDOMRender
}): {
hoverifier: Hoverifier<RepoSpec & RevSpec & FileSpec & ResolvedRevSpec, HoverMerged, ActionItemAction>
subscription: Unsubscribable
} {
Expand Down Expand Up @@ -357,7 +360,12 @@ export function handleCodeHost({
showGlobalDebug,
sourcegraphURL,
telemetryService,
}: CodeIntelligenceProps & { mutations: Observable<MutationRecordLike[]>; sourcegraphURL: string }): Subscription {
render,
}: CodeIntelligenceProps & {
mutations: Observable<MutationRecordLike[]>
sourcegraphURL: string
render: typeof reactDOMRender
}): Subscription {
const history = H.createBrowserHistory()
const subscriptions = new Subscription()
const { requestGraphQL } = platformContext
Expand All @@ -382,6 +390,7 @@ export function handleCodeHost({
extensionsController,
platformContext,
telemetryService,
render,
})
subscriptions.add(hoverifier)
subscriptions.add(subscription)
Expand All @@ -401,6 +410,7 @@ export function handleCodeHost({
history,
platformContext,
telemetryService,
render,
...codeHost.commandPaletteClassProps,
})
)
Expand All @@ -412,7 +422,7 @@ export function handleCodeHost({
// so we don't need to subscribe to mutations.
if (showGlobalDebug) {
const mount = createGlobalDebugMount()
renderGlobalDebug({ extensionsController, platformContext, history, sourcegraphURL })(mount)
renderGlobalDebug({ extensionsController, platformContext, history, sourcegraphURL, render })(mount)
}

// Render view on Sourcegraph button
Expand Down Expand Up @@ -688,6 +698,7 @@ export async function injectCodeIntelligenceToCodeHost(
showGlobalDebug,
sourcegraphURL,
telemetryService,
render: reactDOMRender,
})
)
return subscriptions
Expand Down
3 changes: 3 additions & 0 deletions browser/src/libs/code_intelligence/extensions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,13 @@ interface InjectProps
extends PlatformContextProps<'forceUpdateTooltip' | 'sideloadedExtensionURL'>,
ExtensionsControllerProps {
history: H.History
render: typeof render
}

export const renderCommandPalette = ({
extensionsController,
history,
render,
...props
}: TelemetryProps & InjectProps & Pick<CommandListPopoverButtonProps, 'inputClassName' | 'popoverClassName'>) => (
mount: HTMLElement
Expand All @@ -70,6 +72,7 @@ export const renderGlobalDebug = ({
extensionsController,
platformContext,
history,
render,
sourcegraphURL,
}: InjectProps & { sourcegraphURL: string; showGlobalDebug?: boolean }) => (mount: HTMLElement): void => {
render(
Expand Down
8 changes: 0 additions & 8 deletions browser/src/libs/code_intelligence/text_fields.test.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
const RENDER = jest.fn()
jest.mock('react-dom', () => ({
createPortal: jest.fn(el => el),
render: RENDER,
unmountComponentAtNode: jest.fn(),
}))

import { uniqueId } from 'lodash'
import { from, NEVER, Subject, Subscription } from 'rxjs'
import { first, skip, take } from 'rxjs/operators'
Expand Down Expand Up @@ -35,7 +28,6 @@ describe('text_fields', () => {
let subscriptions = new Subscription()

afterEach(() => {
RENDER.mockClear()
subscriptions.unsubscribe()
subscriptions = new Subscription()
})
Expand Down

0 comments on commit bf1c24d

Please sign in to comment.