elastic / elasticsearch

Free and Open Source, Distributed, RESTful Search Engine
https://www.elastic.co/products/elasticsearch
Other
69.94k stars 24.74k forks source link

Comprehensive aggregation REST tests #26220

Open polyfractal opened 7 years ago

polyfractal commented 7 years ago

There are only a few REST tests that verify the body of aggregation results. We should have a more comprehensive suite of tests that check the body under a variety of conditions and options.

Metrics:

Bucketing

Pipeline

javanna commented 7 years ago

Question: do we want to have these as yaml tests so the language clients can run them too? Or shall we consider making them java tests that se the low-level REST client instead?

polyfractal commented 7 years ago

I was targeting the yaml tests so that clients could also run them, figured that would provide the widest benefit to new tests (since basically none of the clients do any body testing at all, it may smoke out some issues). And yaml tests are sometimes easier to write :)

But I could be convinced either way. What do you think?

javanna commented 7 years ago

I am fine with moving forward with yaml tests, I don't have a strong opinion, I just wanted to make sure that we consider writing java tests too.

markharwood commented 6 years ago

cc @elastic/es-search-aggs

imotov commented 4 years ago

@polyfractal should we assign these as well after we are done with the issue spring cleaning? :smile_cat:

imotov commented 3 years ago

I opened #71303, which hopefully will make writing these tests a bit less awkward.

nik9000 commented 3 years ago

One of the things I've been trying to do when I build these tests is assert that the we take fast paths when its important. Like, when we expect, say, range to get rewritten into a filter-by-filter style filters agg I'm writing a test with that uses the profiler results to assert that our debug output says we hit the fast path. Its not important to do in lots of cases, but, like, when I've spent several days writing fancy fast paths for stuff like range and terms I want to be sure we're not accidentally disabling them. And REST tests are the only place I can be sure of that.

chen-ni commented 2 years ago

Hi @polyfractal, I'd like to work on adding tests but I have a question. Why does every yml file start with an (seemingly) arbitrary number? Like 220_filters_bucket.yml, 230_composite.yml, etc. How do I come up with the number when adding a test file?

not-napoleon commented 2 years ago

The numbering is fairly arbitrary. Feel free to just pick the next available multiple of ten and start there. I believe there are historical reasons for it, but at this point it's mostly tradition. Thanks for helping out!

nik9000 commented 2 years ago

I believe there are historical reasons for it, but at this point it's mostly tradition.

Reminds me of writing basic in middle school.

not-napoleon commented 2 years ago

Reminds me of writing basic in middle school.

I can't tell if that's a point in favor of or against continuing the practice.

elasticsearchmachine commented 2 years ago

Pinging @elastic/es-analytics-geo (Team:Analytics)

elasticsearchmachine commented 3 months ago

Pinging @elastic/es-analytical-engine (Team:Analytics)