Nested sourced from acceptance test cleanup#1311
Conversation
myronmarston
left a comment
myronmarston
left a comment
There was a problem hiding this comment.
Looking good! I left a couple more suggestions that would be good to include when you merge.
| source = fetch_team_source("t1") | ||
| coaches_by_id = coaches_by_id_in(source) | ||
| expect(coaches_by_id.fetch("c1")).to eq("id" => "c1", "name" => "Alice", "salary" => 200) | ||
| # The sibling coach was never sourced, so `salary` is absent and applying c1's events leaves it alone. |
There was a problem hiding this comment.
| # The sibling coach was never sourced, so `salary` is absent and applying c1's events leaves it alone. | |
| # c2's profile was never sourced, so `salary` is absent and applying c1's events leaves it alone. |
| expect(source.fetch(LIST_COUNTS_FIELD)).to include("staff|coaches" => 2) | ||
| end | ||
|
|
||
| it "ignores a stale (lower-version) source event for an already-sourced nested element" do |
There was a problem hiding this comment.
| it "ignores a stale (lower-version) source event for an already-sourced nested element" do | |
| it "ignores a stale (event.version <= stored.version) source event for an already-sourced nested element" do |
|
|
||
| indexer.processor.process([team], refresh_indices: true) | ||
| indexer.processor.process([coach_profile_event("c1", 200, id: "prof-c1", version: 5)], refresh_indices: true) | ||
| indexer.processor.process([coach_profile_event("c1", 100, id: "prof-c1", version: 2)], refresh_indices: true) |
There was a problem hiding this comment.
| indexer.processor.process([coach_profile_event("c1", 100, id: "prof-c1", version: 2)], refresh_indices: true) | |
| indexer.processor.process([ | |
| coach_profile_event("c1", 100, id: "prof-c1", version: 2), # ignored since 2 <= 5 | |
| coach_profile_event("c1", 300, id: "prof-c1", version: 5) # ignored since 5 <= 5 | |
| ], refresh_indices: true) |
To go along with my suggested test description change, would be good to also cover the event.version == stored.version case.
| expect(coaches_by_id_in(fetch_team_source("t1")).fetch("c3")).to eq("id" => "c3", "name" => "Dana", "salary" => 300) | ||
| end | ||
|
|
||
| it "materializes a ghost document from a source event targeting a singleton object path" do |
There was a problem hiding this comment.
Elsewhere we call this an "incomplete document" (compare a search for incomplete_doc vs ghost...).
Would be good to use the existing term (here and anywhere else you use that term).
I do kinda like "ghost document" and am open to switching our terminology to it, but I'd want that to be a separate PR.
|
|
||
| # The full ghost document: no `staff` (or any other team field) yet--just the identity, bookkeeping, | ||
| # and the buffered salary awaiting its target element. The ghost is the one document small enough to | ||
| # pin in full, which also pins the `__nested_sourced_data` buffer entry (real docs carry it too--it's |
There was a problem hiding this comment.
I don't understand what "small enough to pin in full" means. In what sense are we "pinning" anything here? And what does document size have to do with it?
I think this paragraph can be de-slopified and shortened significantly.
| "21:staff.coaches.profile|5:staff|2:cø|" => {"prof-cø" => 12}, | ||
| "29:staff.general_manager.profile|5:staff|15:general_manager|" => {"prof-gm1" => 13} | ||
| ) | ||
|
|
There was a problem hiding this comment.
| # The top-level team event has no nested sourced fields, so `__self` should not get added to `__nested_sourced_data` even | |
| # though `__self` shows up in `__versions`. | |
| expect(source.fetch("__nested_sourced_data").keys).to match_array(source.fetch("__versions").except("__self").keys) | |
This covers the optimization we have have here:
(Without it, I think nothing fails if we remove that guard).
2 participants
Addresses the review threads on #1252 that remained open after it merged, specifically pertaining to the acceptance tests.
This fills the coverage gaps identified in review — mostly around the
__nested_sourced_databuffer's edge cases — and reworks the spec for readability, so that every detail a test's assertions depend on is visible in the test body rather than hidden in helper defaults.