paul-butcher
commented
11 months ago Argh! It looks like the mocking in FiltersAndAggregationsBuilderTest has been bypassing some of the behaviour in FAAB.
There was some "pass through because we don't know what else to do with it" behaviour, which I replaced with E_NOIMPL to be more clear about exactly what this bit does, and to prevent surprises if we do start trying to implement pairing on different kinds of aggregations/filters. The tests were relying on this passthrough behaviour.
This is just refactoring. I believe it improves the clarity of what is going on with the paired aggregations. It also updates the old docstring that I hadn't fixed when I removed the global agg business.
In doing this, I discovered that because of the way we were mocking the filters and aggregation creation in FiltersAndAggregationsBuilderTest, some of the paths the tests were executing were never used in Real Life, so I've changed the way those tests work, introducing a matcher.