Jeroen-G / Explorer

🗺️ Next-gen Elasticsearch driver for Laravel Scout.
https://jeroen-g.github.io/Explorer/
MIT License
358 stars 62 forks source link

Mocking multiple ES responses #190

Open a-drew opened 1 year ago

a-drew commented 1 year ago

I've found an issue while setting up tests and trying to retrieve multiple responses after mocking the results. No matter how you set it up, your response will be provided only the first time, any additional requests to search will fail with the following error: ErrorException : Undefined array key "hits"

Here's example of a failing test:

    public function it_can_lazy_map_results_into_multiple_models(): void
    {
        $file = Storage::disk('test')->readStream('elastic-response-multi-index.json');
        $this->swap(
            ElasticClientFactory::class,
            ElasticClientFactory::fake(new FakeResponse(200, $file))
        );
        $models = Post::search('abc')->join(Media::class)->cursor(); // the es response matches my file
        $this->assertCount(2, $models->where(fn($model) => $model instanceof Post));
        $this->assertCount(2, $models->where(fn($model) => $model instanceof Media));

        $models = Media::search('abc')->join(Post::class)->cursor(); //the es response is completely empty
        $this->assertCount(2, $models->where(fn($model) => $model instanceof Media));
        $this->assertCount(2, $models->where(fn($model) => $model instanceof Post));
    }

After a bit of digging I found this to be a known issue upsteam (https://github.com/elastic/elasticsearch-php/issues/1043), however I did find this workaround:

$this->partialMock(DocumentAdapterInterface::class)
    ->allows('search')
    ->andReturns(new Results([
        'took' => 28,
        'timed_out' => false,
        '_shards' => ['total' => 1, 'successful' => 1, 'skipped' => 0, 'failed' => 0],
        'hits' => ['total' => ['value' => 0], 'max_score' => null, 'hits' => [
            // your results go here
        ]],
    ]));

...

If you find it useful, this could be added to the docs or even replace the current file based approach.

Jeroen-G commented 1 year ago

It depends on what you want to test. I like the file based way because it is an actual ES response, so we can't make the wrong assumptions about it. In other scenarios I can imagine you just want the adapter to return a certain set of assumed results.

For now I think this issue will serve as good enough documentation until more people raise similar issues. Thanks for investigating and your elaborate description (as always 😄)!

a-drew commented 1 year ago

This sounds good to me, most of my tests worked fine with the file approach this was really an outlier.

I'll also note that its possible to use files in this workaround, albeit you're just passing json around and skip both guzzle and the whole ES client mock.

$file = Storage::disk('test')->get('elastic-response-multi-index.json');

$this->partialMock(DocumentAdapterInterface::class)
    ->allows('search')
    ->andReturns(new Results(json_decode($file, true)));