Add integration test for filter by and change CramAdapter to for-of l…

…oop to avoid

allocations

packages/core/ui/Dialog.tsx

@@ -2,9 +2,9 @@ import React from 'react'
 import {
   Dialog,
   DialogTitle,
-  IconButton,
-  Divider,
   DialogProps,
+  Divider,
+  IconButton,
   ScopedCssBaseline,
   ThemeProvider,
   createTheme,

plugins/alignments/src/BamAdapter/BamAdapter.ts

@@ -8,10 +8,10 @@ import { bytesForRegions, updateStatus, Feature } from '@jbrowse/core/util'
 import { openLocation } from '@jbrowse/core/util/io'
 import { ObservableCreate } from '@jbrowse/core/util/rxjs'
 import { toArray } from 'rxjs/operators'
+import { firstValueFrom } from 'rxjs'
 
 // locals
 import BamSlightlyLazyFeature from './BamSlightlyLazyFeature'
-import { firstValueFrom } from 'rxjs'
 
 interface Header {
   idToName: string[]
@@ -203,17 +203,13 @@ export default class BamAdapter extends BaseFeatureDataAdapter {
         }
 
         const flags = record.flags
-        if (
-          !((flags & flagInclude) === flagInclude && !(flags & flagExclude))
-        ) {
+        if ((flags & flagInclude) !== flagInclude && !(flags & flagExclude)) {
           continue
         }
 
         if (tagFilter) {
-          const val = record.get(tagFilter.tag)
-          if (
-            !(val === '*' ? val !== undefined : `${val}` === tagFilter.value)
-          ) {
+          const v = record.get(tagFilter.tag)
+          if (!(v === '*' ? v !== undefined : `${v}` === tagFilter.value)) {
             continue
           }
         }

plugins/alignments/src/CramAdapter/CramAdapter.ts

@@ -246,25 +246,28 @@ export default class CramAdapter extends BaseFeatureDataAdapter {
         readName,
       } = filterBy || {}
 
-      let filtered = records.filter(record => {
+      for (const record of records) {
         const flags = record.flags
-        return (flags & flagInclude) === flagInclude && !(flags & flagExclude)
-      })
-
-      if (tagFilter) {
-        filtered = filtered.filter(record => {
-          const val = record.tags[tagFilter.tag]
-          return val === '*' ? val !== undefined : `${val}` === tagFilter.value
-        })
-      }
+        if ((flags & flagInclude) !== flagInclude && !(flags & flagExclude)) {
+          continue
+        }
+
+        if (tagFilter) {
+          const v =
+            tagFilter.tag === 'RG'
+              ? this.samHeader.readGroups?.[record.readGroupId]
+              : record.tags[tagFilter.tag]
+          if (!(v === '*' ? v !== undefined : `${v}` === tagFilter.value)) {
+            continue
+          }
+        }
 
-      if (readName) {
-        filtered = filtered.filter(record => record.readName === readName)
+        if (readName && record.readName !== readName) {
+          continue
+        }
+        observer.next(this.cramRecordToFeature(record))
       }
 
-      filtered.forEach(record => {
-        observer.next(this.cramRecordToFeature(record))
-      })
       statusCallback('')
       observer.complete()
     }, signal)

plugins/alignments/src/CramAdapter/CramSlightlyLazyFeature.ts

@@ -47,8 +47,7 @@ export default class CramSlightlyLazyFeature implements Feature {
   }
 
   _read_group_id() {
-    const rg = this._store.samHeader.readGroups
-    return rg ? rg[this.record.readGroupId] : undefined
+    return this._store.samHeader.readGroups?.[this.record.readGroupId]
   }
 
   _get_qual() {

plugins/alignments/src/LinearPileupDisplay/components/ColorByTag.tsx

@@ -32,12 +32,10 @@ function ColorByTagDlg(props: {
           placeholder="Enter tag name"
           inputProps={{
             maxLength: 2,
-            'data-testid': 'color-tag-name-input',
           }}
           error={tag.length === 2 && !validTag}
           helperText={tag.length === 2 && !validTag ? 'Not a valid tag' : ''}
           autoComplete="off"
-          data-testid="color-tag-name"
         />
         <DialogActions>
           <Button

plugins/alignments/src/PileupRenderer/components/PileupRendering.tsx

@@ -17,6 +17,7 @@ function PileupRendering(props: {
   bpPerPx: number
   sortedBy?: { type: string; pos: number; refName: string }
   colorBy?: { type: string; tag?: string }
+  filterBy?: { tagFilter?: { tag: string } }
   onMouseMove?: (event: React.MouseEvent, featureId?: string) => void
 }) {
   const {
@@ -29,6 +30,7 @@ function PileupRendering(props: {
     bpPerPx,
     sortedBy,
     colorBy,
+    filterBy,
   } = props
   const { selectedFeatureId, featureIdUnderMouse, contextMenuFeature } =
     displayModel
@@ -167,7 +169,12 @@ function PileupRendering(props: {
   // need to call this in render so we get the right observer behavior
   return (
     <div
-      data-testid={`pileup-${[sortedBy?.type, colorBy?.type, colorBy?.tag]
+      data-testid={`pileup-${[
+        sortedBy?.type,
+        colorBy?.type,
+        colorBy?.tag,
+        filterBy?.tagFilter?.tag,
+      ]
         .filter(f => !!f)
         .join('-')}`}
       style={{ position: 'relative', width: canvasWidth, height }}

plugins/alignments/src/shared/FilterByTag.tsx

@@ -69,27 +69,27 @@ function Bitmask(props: { flag?: number; setFlag: Function }) {
     </>
   )
 }
-
+interface FilterBy {
+  flagExclude: number
+  flagInclude: number
+  readName?: string
+  tagFilter?: { tag: string; value: string }
+}
 function FilterByTagDlg(props: {
   model: {
-    filterBy?: {
-      flagExclude: number
-      flagInclude: number
-      readName?: string
-      tagFilter?: { tag: string; value: string }
-    }
-    setFilterBy: Function
+    filterBy: FilterBy
+    setFilterBy: (arg: FilterBy) => void
   }
   handleClose: () => void
 }) {
   const { model, handleClose } = props
   const { classes } = useStyles()
   const { filterBy } = model
-  const [flagInclude, setFlagInclude] = useState(filterBy?.flagInclude)
-  const [flagExclude, setFlagExclude] = useState(filterBy?.flagExclude)
-  const [tag, setTag] = useState(filterBy?.tagFilter?.tag || '')
-  const [tagValue, setTagValue] = useState(filterBy?.tagFilter?.value || '')
-  const [readName, setReadName] = useState(filterBy?.readName || '')
+  const [flagInclude, setFlagInclude] = useState(filterBy.flagInclude)
+  const [flagExclude, setFlagExclude] = useState(filterBy.flagExclude)
+  const [tag, setTag] = useState(filterBy.tagFilter?.tag || '')
+  const [tagValue, setTagValue] = useState(filterBy.tagFilter?.value || '')
+  const [readName, setReadName] = useState(filterBy.readName || '')
   const validTag = tag.match(/^[A-Za-z][A-Za-z0-9]$/)
 
   const site = 'https://broadinstitute.github.io/picard/explain-flags.html'
@@ -127,21 +127,15 @@ function FilterByTagDlg(props: {
             placeholder="Enter tag name"
             inputProps={{
               maxLength: 2,
-              'data-testid': 'color-tag-name-input',
             }}
             error={tag.length === 2 && !validTag}
             helperText={tag.length === 2 && !validTag ? 'Not a valid tag' : ''}
-            data-testid="color-tag-name"
           />
           <TextField
             className={classes.field}
             value={tagValue}
             onChange={event => setTagValue(event.target.value)}
             placeholder="Enter tag value"
-            inputProps={{
-              'data-testid': 'color-tag-name-input',
-            }}
-            data-testid="color-tag-value"
           />
         </Paper>
         <Paper className={classes.paper} variant="outlined">
@@ -151,10 +145,6 @@ function FilterByTagDlg(props: {
             value={readName}
             onChange={event => setReadName(event.target.value)}
             placeholder="Enter read name"
-            inputProps={{
-              'data-testid': 'color-tag-readname-input',
-            }}
-            data-testid="color-tag-readname"
           />
         </Paper>
         <DialogActions>

products/jbrowse-web/src/tests/AlignmentsFeatures.test.tsx

@@ -1,4 +1,5 @@
-import { fireEvent, within } from '@testing-library/react'
+import { screen, within } from '@testing-library/react'
+import userEvent from '@testing-library/user-event'
 
 // locals
 import {
@@ -17,74 +18,97 @@ beforeEach(() => {
   doBeforeEach()
 })
 
-const delay = { timeout: 20000 }
+const delay = { timeout: 30000 }
 const opts = [{}, delay]
 
 test('opens the track menu and enables soft clipping', async () => {
-  const { view, findByTestId, findByText } = await createView()
-  await findByText('Help')
+  const user = userEvent.setup()
+  const { view } = await createView()
   view.setNewView(0.02, 142956)
 
-  // load track
-  fireEvent.click(await findByTestId(hts('volvox-long-reads-sv-bam'), ...opts))
-
-  // opens the track menu
-  fireEvent.click(await findByTestId('track_menu_icon', ...opts))
-  fireEvent.click(await findByText('Show soft clipping'))
-
-  // wait for block to rerender
-  const f0 = within(await findByTestId('Blockset-pileup'))
-
+  await user.click(
+    await screen.findByTestId(hts('volvox-long-reads-sv-bam'), ...opts),
+  )
+  await user.click(await screen.findByTestId('track_menu_icon', ...opts))
+  await user.click(await screen.findByText('Show soft clipping'))
+  const f0 = within(await screen.findByTestId('Blockset-pileup'))
   // slightly higher threshold for fonts
   expectCanvasMatch(
     await f0.findByTestId(pc('softclipped_{volvox}ctgA:2849..2864-0'), ...opts),
     0.05,
   )
-}, 30000)
+}, 50000)
 
 test('selects a sort, sort by base pair', async () => {
-  const { view, findByTestId, findByText, findAllByTestId } = await createView()
-  await findByText('Help')
+  const user = userEvent.setup()
+  const { view } = await createView()
   view.setNewView(0.043688891869634636, 301762)
-  const track = 'volvox_cram_alignments_ctga'
 
   // load track
-  fireEvent.click(await findByTestId(hts(track), ...opts))
-
-  fireEvent.click(await findByTestId('track_menu_icon', ...opts))
-  fireEvent.click(await findByText('Sort by'))
-  fireEvent.click(await findByText('Base pair'))
-
-  // wait for pileup track to render with sort
-  await findAllByTestId('pileup-Base pair', ...opts)
-  const f1 = within(await findByTestId('Blockset-pileup'))
+  await user.click(
+    await screen.findByTestId(hts('volvox_cram_alignments_ctga'), ...opts),
+  )
+  await user.click(await screen.findByTestId('track_menu_icon', ...opts))
+  await user.click(await screen.findByText('Sort by'))
+  await user.click(await screen.findByText('Base pair'))
+  await screen.findAllByTestId('pileup-Base pair', ...opts)
+  const f1 = within(await screen.findByTestId('Blockset-pileup'))
   expectCanvasMatch(await f1.findByTestId(pv('13196..13230-0'), ...opts))
-
-  // maintains sort after zoom out
-  fireEvent.click(await findByTestId('zoom_out'))
-  await findAllByTestId('pileup-Base pair', ...opts)
-  const f2 = within(await findByTestId('Blockset-pileup'))
+  await user.click(await screen.findByTestId('zoom_out'))
+  await screen.findAllByTestId('pileup-Base pair', ...opts)
+  const f2 = within(await screen.findByTestId('Blockset-pileup'))
   expectCanvasMatch(await f2.findByTestId(pv('13161..13230-0'), ...opts))
 }, 35000)
 
 test('color by tag', async () => {
-  const { view, findByTestId, findByText, findAllByTestId } = await createView()
-  await findByText('Help')
+  const user = userEvent.setup()
+  const { view } = await createView()
   view.setNewView(0.465, 85055)
-
-  // load track
-  fireEvent.click(await findByTestId(hts('volvox_cram'), ...opts))
-
-  // colors by HP tag
-  fireEvent.click(await findByTestId('track_menu_icon', ...opts))
-  fireEvent.click(await findByText('Color scheme'))
-  fireEvent.click(await findByText('Color by tag...'))
-  fireEvent.change(await findByTestId('color-tag-name-input'), {
-    target: { value: 'HP' },
-  })
-  fireEvent.click(await findByText('Submit'))
-  // wait for pileup track to render with color
-  await findAllByTestId('pileup-tag-HP', ...opts)
-  const f1 = within(await findByTestId('Blockset-pileup'))
+  await user.click(await screen.findByTestId(hts('volvox_cram'), ...opts))
+  await user.click(await screen.findByTestId('track_menu_icon', ...opts))
+  await user.click(await screen.findByText('Color scheme'))
+  await user.click(await screen.findByText('Color by tag...'))
+  await user.type(await screen.findByPlaceholderText('Enter tag name'), 'HP')
+  await user.click(await screen.findByText('Submit'))
+  await screen.findAllByTestId('pileup-tag-HP', ...opts)
+  const f1 = within(await screen.findByTestId('Blockset-pileup'))
   expectCanvasMatch(await f1.findByTestId(pv('39805..40176-0'), ...opts))
-}, 30000)
+}, 50000)
+
+async function testFilterTrack(
+  trackId: string,
+  tag: string,
+  value: string,
+  key: string,
+) {
+  const user = userEvent.setup()
+  await user.click(await screen.findByTestId(hts(trackId), ...opts))
+  await user.click(await screen.findByTestId('track_menu_icon', ...opts))
+  await user.click(await screen.findByText('Filter by'))
+  await user.type(await screen.findByPlaceholderText('Enter tag name'), tag)
+  await user.type(await screen.findByPlaceholderText('Enter tag value'), value)
+  await user.click(await screen.findByText('Submit'))
+  await screen.findAllByTestId(`pileup-${tag}`, ...opts)
+  const f1 = within(await screen.findByTestId('Blockset-pileup'))
+  expectCanvasMatch(await f1.findByTestId(pv(key), ...opts))
+}
+
+test('filter by HP tag cram', async () => {
+  const { view } = await createView()
+  view.setNewView(0.465, 85055)
+  await testFilterTrack('volvox_cram', 'HP', '1', '39805..40176-0')
+}, 50000)
+
+test('filter by HP tag bam', async () => {
+  const { view } = await createView()
+  view.setNewView(0.465, 85055)
+  await testFilterTrack('volvox_bam', 'HP', '1', '39805..40176-0')
+}, 50000)
+
+// snapshot not working but appears to work in browser
+xtest('filter by RG tag cram (special case tag))', async () => {
+  const { container, view } = await createView()
+  await view.navToLocString('ctgA:1000..2000')
+  await testFilterTrack('volvox_cram', 'RG', '6', '1002..2002-0')
+  expect(container).toMatchSnapshot()
+}, 50000)