Skip to content

Commit

Permalink
Fix flakyness of connection test (#3630)
Browse files Browse the repository at this point in the history
* Connection type changes

* Misc typescript fixes to fetch mocks
  • Loading branch information
cmdcolin committed May 18, 2023
1 parent e514be4 commit 47002c0
Show file tree
Hide file tree
Showing 21 changed files with 791 additions and 937 deletions.
4 changes: 3 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@
"@typescript-eslint/no-var-requires": "off",
"@typescript-eslint/no-unsafe-member-access": "off",
"@typescript-eslint/no-unsafe-argument": "off",

"@typescript-eslint/no-unsafe-assignment": "off",
"@typescript-eslint/no-unsafe-call": "off",
"@typescript-eslint/no-unsafe-return": "off",

"testing-library/render-result-naming-convention": 0,
"testing-library/prefer-screen-queries": 0,

"unicorn/no-new-array": 0,
"unicorn/prefer-type-error": 0,
"unicorn/prefer-node-protocol": 0,
Expand Down
10 changes: 0 additions & 10 deletions config/jest/createRange.js

This file was deleted.

1 change: 0 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ module.exports = {
],
setupFiles: [
'<rootDir>/config/jest/textEncoder.js',
'<rootDir>/config/jest/createRange.js',
'<rootDir>/config/jest/fetchMock.js',
'<rootDir>/config/jest/console.js',
'<rootDir>/config/jest/crypto.js',
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
"@storybook/react-webpack5": "^7.0.0-beta.35",
"@testing-library/jest-dom": "^5.16.5",
"@testing-library/react": "^12.0.0",
"@testing-library/user-event": "^14.4.3",
"@types/base64-js": "^1.2.5",
"@types/buffer-crc32": "^0.2.2",
"@types/cli-progress": "^3.9.2",
Expand Down
3 changes: 3 additions & 0 deletions packages/__mocks__/@jbrowse/core/ui/SanitizedHTML.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function SanitizedHTML({ html }: { html: string }) {
return html
}
4 changes: 4 additions & 0 deletions packages/core/PluginManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ export default class PluginManager {
return this.getElementTypesInGroup('track') as TrackType[]
}

getConnectionElements() {
return this.getElementTypesInGroup('connection') as ConnectionType[]
}

getAddTrackWorkflowElements() {
return this.getElementTypesInGroup(
'add track workflow',
Expand Down
6 changes: 6 additions & 0 deletions packages/core/ui/SanitizedHTML.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ export function isHTML(str: string) {
return full.test(str)
}

// note this is mocked during testing, see
// packages/__mocks__/@jbrowse/core/ui/SanitizedHTML something about dompurify
// behavior causes errors during tests, was seen in
// products/jbrowse-web/src/tests/Connection.test.tsx test (can delete mock to
// see)
//
export default function SanitizedHTML({ html }: { html: string }) {
const value = isHTML(html) ? html : escapeHTML(html)
if (!added) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1262,6 +1262,7 @@ exports[`ConfigurationEditor widget renders with defaults of the PileupTrack sch
</div>
<input
aria-hidden="true"
aria-invalid="false"
class="MuiSelect-nativeInput css-yf8vq0-MuiSelect-nativeInput"
tabindex="-1"
value="PileupRenderer"
Expand Down Expand Up @@ -1427,6 +1428,7 @@ exports[`ConfigurationEditor widget renders with defaults of the PileupTrack sch
</div>
<input
aria-hidden="true"
aria-invalid="false"
class="MuiSelect-nativeInput css-yf8vq0-MuiSelect-nativeInput"
tabindex="-1"
value="fr"
Expand Down Expand Up @@ -1503,6 +1505,7 @@ exports[`ConfigurationEditor widget renders with defaults of the PileupTrack sch
</div>
<input
aria-hidden="true"
aria-invalid="false"
class="MuiSelect-nativeInput css-yf8vq0-MuiSelect-nativeInput"
tabindex="-1"
value="normal"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import AddConnectionWidget from './AddConnectionWidget'

jest.mock('@jbrowse/web/src/makeWorkerInstance', () => () => {})

// window.fetch = jest.fn(() => new Promise(resolve => resolve()))

describe('<AddConnectionWidget />', () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
let model: any
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,14 @@
import React, { useState } from 'react'
import { Button, Step, StepContent, StepLabel, Stepper } from '@mui/material'
import { getSession, getEnv } from '@jbrowse/core/util'
import {
Button,
Step,
StepContent,
StepLabel,
Stepper,
Typography,
} from '@mui/material'
import { makeStyles } from 'tss-react/mui'
import { observer } from 'mobx-react'
import { AnyConfigurationModel } from '@jbrowse/core/configuration'
import { ConnectionType } from '@jbrowse/core/pluggableElementTypes'

// locals
import ConfigureConnection from './ConfigureConnection'
import ConnectionTypeSelect from './ConnectionTypeSelect'
import { AnyConfigurationModel } from '@jbrowse/core/configuration'

const useStyles = makeStyles()(theme => ({
root: {
Expand All @@ -35,82 +28,18 @@ const useStyles = makeStyles()(theme => ({

const steps = ['Select a Connection Type', 'Configure Connection']

function AddConnectionWidget({ model }: { model: unknown }) {
export default observer(function AddConnectionWidget({
model,
}: {
model: unknown
}) {
const [connectionType, setConnectionType] = useState<ConnectionType>()
const [configModel, setConfigModel] = useState<AnyConfigurationModel>()
const [activeStep, setActiveStep] = useState(0)
const { classes } = useStyles()

const session = getSession(model)

const { pluginManager } = getEnv(session)

function stepContent() {
switch (activeStep) {
case 0:
return (
<ConnectionTypeSelect
connectionTypeChoices={
pluginManager.getElementTypesInGroup(
'connection',
) as ConnectionType[]
}
connectionType={connectionType}
setConnectionType={c => {
setConnectionType(c)
if (c) {
setConfigModel(
c.configSchema.create(
{
connectionId: `${c.name}-${Date.now()}`,
},
getEnv(model),
),
)
}
}}
/>
)
case 1:
return connectionType && configModel ? (
<ConfigureConnection
connectionType={connectionType}
model={configModel}
session={session}
/>
) : null

default:
return <Typography>Unknown step</Typography>
}
}

function handleNext() {
if (activeStep === steps.length - 1) {
handleFinish()
} else {
setActiveStep(activeStep + 1)
}
}

function handleBack() {
setActiveStep(activeStep - 1)
}

function handleFinish() {
const connectionConf = session.addConnectionConf(configModel)
if (session.makeConnection) {
session.makeConnection(connectionConf)
}
session.hideWidget(model)
}

function checkNextEnabled() {
return (
(activeStep === 0 && connectionType) || (activeStep === 1 && configModel)
)
}

return (
<div className={classes.root}>
<Stepper
Expand All @@ -122,20 +51,54 @@ function AddConnectionWidget({ model }: { model: unknown }) {
<Step key={label}>
<StepLabel>{label}</StepLabel>
<StepContent>
{stepContent()}
{activeStep === 0 ? (
<ConnectionTypeSelect
connectionTypeChoices={pluginManager.getConnectionElements()}
connectionType={connectionType}
setConnectionType={c => {
setConnectionType(c)
if (!c) {
return
}
const connectionId = `${c.name}-${Date.now()}`
setConfigModel(
c.configSchema.create({ connectionId }, getEnv(model)),
)
}}
/>
) : connectionType && configModel ? (
<ConfigureConnection
connectionType={connectionType}
model={configModel}
session={session}
/>
) : null}
<div className={classes.actionsContainer}>
<Button
disabled={activeStep === 0}
onClick={handleBack}
onClick={() => setActiveStep(activeStep - 1)}
className={classes.button}
>
Back
</Button>
<Button
disabled={!checkNextEnabled()}
disabled={
!(
(activeStep === 0 && connectionType) ||
(activeStep === 1 && configModel)
)
}
variant="contained"
color="primary"
onClick={handleNext}
onClick={() => {
if (activeStep === steps.length - 1) {
const conf = session.addConnectionConf(configModel)
session.makeConnection?.(conf)
session.hideWidget(model)
} else {
setActiveStep(activeStep + 1)
}
}}
className={classes.button}
data-testid="addConnectionNext"
>
Expand All @@ -148,6 +111,4 @@ function AddConnectionWidget({ model }: { model: unknown }) {
</Stepper>
</div>
)
}

export default observer(AddConnectionWidget)
})
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,21 @@ import { AnyConfigurationModel } from '@jbrowse/core/configuration'
import { AbstractSessionModel } from '@jbrowse/core/util'
import { LoadingEllipses } from '@jbrowse/core/ui'

const ConfigureConnection = observer(
(props: {
connectionType: ConnectionType
model: AnyConfigurationModel
session: AbstractSessionModel
}) => {
const { connectionType, model, session } = props
const ConfigEditorComponent =
connectionType.configEditorComponent || ConfigurationEditor
export default observer(function ({
connectionType,
model,
session,
}: {
connectionType: ConnectionType
model: AnyConfigurationModel
session: AbstractSessionModel
}) {
const ConfigEditorComponent =
connectionType.configEditorComponent || ConfigurationEditor

return (
<Suspense fallback={<LoadingEllipses />}>
<ConfigEditorComponent model={{ target: model }} session={session} />
</Suspense>
)
},
)

export default ConfigureConnection
return (
<Suspense fallback={<LoadingEllipses />}>
<ConfigEditorComponent model={{ target: model }} session={session} />
</Suspense>
)
})
Original file line number Diff line number Diff line change
@@ -1,17 +1,20 @@
import React, { useEffect } from 'react'
import { IconButton, MenuItem, TextField } from '@mui/material'
import { ConnectionType } from '@jbrowse/core/pluggableElementTypes'
import { observer } from 'mobx-react'

// icons
import OpenInNewIcon from '@mui/icons-material/OpenInNew'

function ConnectionTypeSelect(props: {
export default observer(function ConnectionTypeSelect({
connectionTypeChoices,
connectionType,
setConnectionType,
}: {
connectionTypeChoices: ConnectionType[]
connectionType?: ConnectionType
setConnectionType: (c?: ConnectionType) => void
}) {
const { connectionTypeChoices, connectionType, setConnectionType } = props

useEffect(() => {
if (!connectionType) {
setConnectionType(connectionTypeChoices[0])
Expand Down Expand Up @@ -58,6 +61,4 @@ function ConnectionTypeSelect(props: {
) : null}
</form>
)
}

export default ConnectionTypeSelect
})
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ exports[`<AddConnectionWidget /> renders 1`] = `
</div>
<input
aria-hidden="true"
aria-invalid="false"
class="MuiSelect-nativeInput css-yf8vq0-MuiSelect-nativeInput"
tabindex="-1"
value="UCSCTrackHubConnection"
Expand Down
8 changes: 2 additions & 6 deletions plugins/data-management/src/ucsc-trackhub/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,8 @@ export default function UCSCTrackHubConnection(pluginManager: PluginManager) {
) {
continue
}
const conf = session.assemblies.find(a =>
readConfObject(a, 'name') === genomeName ||
Array.isArray(readConfObject(a, 'aliases'))
? readConfObject(a, 'aliases').includes(genomeName)
: false,
)

const conf = session.assemblyManager.get(genomeName)?.configuration
if (!conf) {
throw new Error(
`Cannot find assembly for "${genomeName}" from the genomes file for connection "${connectionName}"`,
Expand Down
Loading

0 comments on commit 47002c0

Please sign in to comment.