Skip to content

[Enterprise Search] Refactor MockRouter test helper to not store payload#90206

Merged
cee-chen merged 3 commits intoelastic:masterfrom
cee-chen:mock-router-payload
Feb 4, 2021
Merged

[Enterprise Search] Refactor MockRouter test helper to not store payload#90206
cee-chen merged 3 commits intoelastic:masterfrom
cee-chen:mock-router-payload

Conversation

@cee-chen
Copy link
Copy Markdown
Contributor

@cee-chen cee-chen commented Feb 3, 2021 •
edited
Loading

Summary

  1. Past-Constance foolishly assumed we would would only ever have 1 main kind of payload (query, body, or param) to validate
  2. Past-Constance also didn't reeeally know how the router worked
  3. Present-Constance still doesn't, but has improved our MockRouter helper to no longer take a payload param on MockRouter initialization
  4. Instead, when we're validating payloads, we now simply examine the request itself as the payload type(s) are going to be the top-most level keys, e.g.:
{
  query: {
    foo: 'bar',
  },
  body: {
    bar: 'baz',
  }
}

So now Jason no longer needs to create 2 separate routers to test both query and body payloads. Hooray!

Checklist

@cee-chen cee-chen added release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Feb 3, 2021
@cee-chen cee-chen requested review from a team, JasonStoltz and scottybollinger February 3, 2021 17:04
scottybollinger
scottybollinger approved these changes Feb 3, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@scottybollinger scottybollinger left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for doing this!

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Feb 3, 2021

????????????????? WTF is Kibana CI erroring on?

@spalger
Copy link
Copy Markdown
Contributor

spalger commented Feb 3, 2021 •
edited
Loading

@constancecchen sorry about this, we had a little oopsy and all the in progress PRs died

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Feb 3, 2021

HAHA sorry Spencer, I forgot you are an omnipresent wizard. All good and no worries, I just like yelling at bots because they can't yell back (yet...) 🤖

@scottybollinger
Copy link
Copy Markdown
Contributor

scottybollinger commented Feb 3, 2021

@constancesearch yell upstream

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Feb 4, 2021

@elasticmachine merge upstream

1 similar comment
@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Feb 4, 2021

@elasticmachine merge upstream

@cee-chen
Copy link
Copy Markdown
Contributor Author

cee-chen commented Feb 4, 2021

Oh I should actually pay attention to errors someday. Looks like there's new WS tests to updoot

@JasonStoltz
Copy link
Copy Markdown
Member

JasonStoltz commented Feb 4, 2021

Constance, thank you so much 100% awesome.

@cee-chen cee-chen force-pushed the mock-router-payload branch from 59d289a to 495fe39 Compare February 4, 2021 17:51
@kibanamachine
Copy link
Copy Markdown
Contributor

kibanamachine commented Feb 4, 2021

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

  • 💔 Build #103914 failed 59d289a19290b3e825e1fccc11ff34a4dcd743e5
  • 💔 Build #103902 failed 22639b772e8926809b0d09d3163e293898395c6b
  • 💚 Build #103536 succeeded 51001c9fec43e1901e83d7716abb86a3b2823feb

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@cee-chen cee-chen merged commit 2955d65 into elastic:master Feb 4, 2021
@cee-chen cee-chen deleted the mock-router-payload branch February 4, 2021 19:48
@cee-chen cee-chen mentioned this pull request Feb 4, 2021
Merged
cee-chen pushed a commit that referenced this pull request Feb 4, 2021
[Enterprise Search] Refactor MockRouter test helper to not store payl…
da7c3b4 
…oad (#90206) (#90322)

* Update MockRouter to not pass/set a this.payload

- but instead intelligently validate payloads based on the request keys

* Fix relevance tuning API routes to not need a separate mock router for validating query & body

* Update all remaining tests to no longer pass a payload param to MockRouter
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 4, 2021
@gmmorris
Merge branch 'master' into task-manager/support-limited-concurrency
5190abf 
* master: (244 commits)
  [maps] Top hits per entity--change to title to use recent, minor edits (elastic#89254)
  [DOCS] Update installation details (elastic#90354)
  RFC for automatically generated typescript API documentation for every plugins public services, types, and functionality (elastic#86704)
  Elastic Maps Server config is `host` not `hostname` (elastic#90234)
  Use doc link services in index pattern management (elastic#89937)
  [Fleet] Managed Agent Policy (elastic#88688)
  [Workplace Search] Fix Source Settings bug  (elastic#90242)
  [Enterprise Search] Refactor MockRouter test helper to not store payload (elastic#90206)
  Use doc link service in more Stack Monitoring pages (elastic#89050)
  [App Search] Relevance Tuning logic - actions and selectors only, no listeners (elastic#89313)
  Remove UI filters from UI (elastic#89793)
  Use newfeed.service config for all newsfeeds (elastic#90252)
  skip flaky suite (elastic#85086)
  Add readme to geo containment alert covering test alert setup (elastic#89625)
  [APM] Enabling yesterday option when 24 hours is selected (elastic#90017)
  Test user for maps tests under import geoJSON tests (elastic#86015)
  [Lens] Hide column in table (elastic#88680)
  [Security Solution][Detections] Reduce detection engine reliance on _source (elastic#89371)
  [Discover] Minor cleanup (elastic#90260)
  [Search Session][Management] Rename "cancel" button and delete "Reload" button (elastic#90015)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JasonStoltz JasonStoltz JasonStoltz approved these changes

+1 more reviewer

@scottybollinger scottybollinger scottybollinger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

release_note:skip Skip the PR/issue when compiling release notes v7.12.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@cee-chen @spalger @scottybollinger @JasonStoltz @kibanamachine