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

Submit Search Tests and Refactor #1049

Merged
merged 1 commit into from Sep 9, 2020
Merged

Submit Search Tests and Refactor #1049

merged 1 commit into from Sep 9, 2020

Conversation

jameskerr
Copy link
Member

@jameskerr jameskerr commented Sep 3, 2020

Now that we have a properly tested zealot client and an easy way to stub responses, I wrote a series of tests to lock in the behavior we expect from a user hitting "enter" in several scenarios. Once the tests were in place, I pulled out common procedures into their own functions. I'm more pleased with the API for searching within the app now. No new behavior has been added to the app here.

The parts of the app that rely on this code are:

  • Issuing a search for events
  • Issue a search for analytics
  • Issuing a search for index
  • Scrolling down the the bottom of the viewer to issue a "fetch next page" search
  • Killing a search via the three dot menu
  • Populating the uid correlation in the detail pane
  • Populating the hash correlation in the detail pane for a files log
  • The auto-refresh behavior during ingest
  • Clicking on a history entry in the tree and linear view

Example

To issue a search within brim, use the search() function defined in src/js/flows/search/mod.js.

const {response, promise, abort} = search({
  query: "_path=conn"
  from: new Date(123),
  to: new Date(456),
  spaceId: "sp_789,
  id: "Viewer",
  target: "events"
})

The response can be used to setup callbacks for each ndjson payload in the response stream.

response
  .status(status => setStatus(status))
  .chan(0, (records) => setRecords(records))
  .error(e => handleError(e))

The promise is resolved when the request finishes successfully and rejects if there is an error.

promise
  .then(handleSuccess)
  .catch(handleError)
  .finally(cleanup)

The abort function is self-explanitory. It is useful if you are issuing a search within a component and need to provide a cleanup function.

useEffect(() => {
  const {abort, response} = search(args)
  // handle response
  return () => abort() 
})

The Diff

The diff may seem intimidating, but only diffing the changes to source reveals about 600 lines modified. The rest are tests and test snapshots.

$ git diff master...head --stat ':!*test*' ':!*txt'

45 files changed, 536 insertions(+), 596 deletions(-)

 src/js/brim/index.js                                       |   3 -
 src/js/components/FilterTree.js                            |   2 +-
 src/js/components/IngestRefresh.js                         |   4 +-
 src/js/components/Investigation/FindingCard.js             |   2 +-
 src/js/components/LogDetails/Md5Panel.js                   |  17 ++---
 src/js/components/SearchBar/Input.js                       |   2 +-
 src/js/components/SearchBar/Pins.js                        |  68 ++++++------------
 src/js/components/SearchBar/SubmitButton.js                |   2 +-
 src/js/components/SearchBar/mod.js                         |   4 +-
 src/js/components/Span/SpanControls.js                     |   5 +-
 src/js/components/UidTimeline.js                           |   2 +-
 src/js/components/Viewer/HeaderCell.js                     |   2 +-
 src/js/components/charts/MainHistogram/useMainHistogram.js |   2 +-
 src/js/components/useSpanPickerMenu.js                     |   2 +-
 src/js/electron/menu/actions/searchActions.js              |  14 ++--
 src/js/flows/executeHistogramSearch.js                     |  43 -----------
 src/js/flows/executeSearch.js                              |  94 ------------------------
 src/js/flows/executeTableSearch.js                         |  69 ------------------
 src/js/flows/executeUidSearch.js                           |  41 -----------
 src/js/flows/fetchNextPage.js                              |  14 +++-
 src/js/flows/initSpace.js                                  |   4 +-
 src/js/flows/search/handler.js                             |  83 +++++++++++++++++++++
 src/js/flows/search/mod.js                                 |  39 ++++++++++
 src/js/{brim/search.js => flows/search/response.js}        |  32 +++------
 src/js/flows/searches/histogramSearch.js                   |  67 +++++++++++++++++
 src/js/flows/searches/md5Search.js                         |  28 ++++++++
 src/js/flows/searches/uidSearch.js                         |  44 ++++++++++++
 src/js/flows/searches/viewerSearch.js                      |  87 ++++++++++++++++++++++
 src/js/flows/submitAutoRefreshSearch.js                    |  32 ++++-----
 src/js/flows/submitEventsSearch.js                         |  54 --------------
 src/js/flows/submitIndexSearch.js                          | 124 --------------------------------
 src/js/flows/submitSearch.js                               |  22 ------
 src/js/flows/submitSearch/mod.js                           |  53 ++++++++++++++
 src/js/flows/submitSearch/responses/mod.js                 |   8 +++
 src/js/flows/submitSearch/save.js                          |  24 +++++++
 src/js/flows/viewLogDetail.js                              |   4 +-
 src/js/initializers/initNewSearchTab.js                    |   2 +-
 src/js/state/Handlers/selectors.js                         |   5 +-
 src/js/state/Search/reducer.js                             |   5 +-
 src/js/state/Search/types.js                               |   3 +-
 src/js/state/SearchBar/flows.js                            |   4 +-
 src/js/state/Tab/selectors.js                              |   1 -
 src/js/state/Viewer/actions.js                             |   2 +-
 src/js/state/Viewer/types.js                               |   2 +-
 zealot/fetcher/callbacks.ts                                |  11 ++-

@jameskerr jameskerr changed the base branch from master to search-header September 3, 2020 21:09
Base automatically changed from search-header to master September 4, 2020 17:16
@jameskerr jameskerr force-pushed the submit-search-tests branch 3 times, most recently from 3e32a2c to 5d43e59 Compare September 4, 2020 18:49
No longer store last ts of search, use the Last.getSearch function instead
Consolidate common test setup
Convert pins to functional component
We want to save the current program, not previous
Tests for submitSearch
refactored executeSearch
Refactored to use issue and emit
Moved common search things to a search module
Remove old search object
Copy link
Contributor

@mason-fish mason-fish left a comment

Choose a reason for hiding this comment

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

Nice, I dig the new api and all these tests! Also saw some pure -> functional component refactoring in here 👌

@jameskerr jameskerr merged commit 7046367 into master Sep 9, 2020
@jameskerr jameskerr deleted the submit-search-tests branch September 9, 2020 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants