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

Fix flakyness of connection test #3630

Merged
merged 4 commits into from
Apr 6, 2023
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
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',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

createRange polyfill no longer needed, and in fact, causes problems if used

'<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
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated from PR #3625 which actually needed more parenthesizing in it's current form, and should be easier with this method

if (!conf) {
throw new Error(
`Cannot find assembly for "${genomeName}" from the genomes file for connection "${connectionName}"`,
Expand Down
Loading