Adds Coordination Diagnostics Tests#142709
Conversation
Adds CoordinationDiagnosticsDetailsWireSerializingTests, CoordinationDiagnosticsResultWireSerializingTests and CoordinationDiagnosticsStatusWireSerializingTests
joshua-adams-1
added
>test
|
As a note, these tests were added via LLM during spacetime, and since the tests heavily rely on randomisation, the newly added tests have been run a couple of thousand times to ensure there are no failure paths |
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
What's the motivation for the additional/changed testing? |
As part of my SpaceTime project, I was using LLMs to identify testing gaps in the codebase. For want of needing a package to apply this to, I picked
which is always good to have! |
DiannaHohensee
left a comment
DiannaHohensee
left a comment
There was a problem hiding this comment.
Looks fine, made some suggestions. It seems like mutateInstance doesn't need to be implemented for records, so optionally that code could be tossed?
...g/elasticsearch/cluster/coordination/CoordinationDiagnosticsDetailsWireSerializingTests.java
Outdated
Show resolved
Hide resolved
...g/elasticsearch/cluster/coordination/CoordinationDiagnosticsDetailsWireSerializingTests.java
Show resolved
Hide resolved
| } | ||
|
|
||
| @Override | ||
| protected CoordinationDiagnosticsDetails mutateInstance(CoordinationDiagnosticsDetails instance) throws IOException { |
There was a problem hiding this comment.
This patch is refactoring the pre-existing test, so this isn't new, but there is a comment (looks like you added a bit ago) that suggests there's no purpose in implementing mutateInstance for records with default equals() implementations.
I wonder if this logic should be removed, but I don't have much of an opinion. I didn't do code sleuthing, but it's possible that these objects were originally classes and later became records -- we've been slowly converting code to records where possible for a while, AIUI.
There was a problem hiding this comment.
Good catch! I was writing tests via an LLM and despite specifically requesting it didn't implement mutateInstance for records it went and did it anyways! Anyways, I think my job is safe for a while 😆
There was a problem hiding this comment.
Ha, yeah, it did that to me recently, too. "AI" is a probabilistic stats model, i.e. technically random behavior 😅
...rg/elasticsearch/cluster/coordination/CoordinationDiagnosticsResultWireSerializingTests.java
Outdated
Show resolved
Hide resolved
…elasticsearch into coordination-diagnostics
joshua-adams-1
left a comment
joshua-adams-1
left a comment
There was a problem hiding this comment.
Thanks for reviewing! Have addressed all your comments
| } | ||
|
|
||
| @Override | ||
| protected CoordinationDiagnosticsDetails mutateInstance(CoordinationDiagnosticsDetails instance) throws IOException { |
There was a problem hiding this comment.
Good catch! I was writing tests via an LLM and despite specifically requesting it didn't implement mutateInstance for records it went and did it anyways! Anyways, I think my job is safe for a while 😆
…locations * upstream/main: (126 commits) Update KnnIndexTester to use more settings from datasets (elastic#143869) fix: dynamic template vector array is overridden by automatic dense_vector mapping (elastic#143733) ES|QL: Don't reuse the same alias for _fork column (elastic#143909) Close and initialize clients after each node upgrade in logsdb rolling upgrade tests. (elastic#143823) ESQL: Added GroupedTopNOperator for LIMIT BY, compute only (elastic#143476) Handle views in ResolveIndexAction (elastic#143561) Improve reindex rethrottle API in stateless (elastic#143771) Use a copy of the SearchExecutionContext for each Percolator execution (elastic#142765) Log the stacktrace when we encounter a deprecation warning for `default_metric` (elastic#143929) ESQL: evaluate ReferenceAttributes to potentially FieldAttributes for full-text functions restriction (elastic#143893) Add ClusterStateSerializationStats Serializatation Tests (elastic#142703) Adds Coordination Diagnostics Tests (elastic#142709) Upgrade Elasticsearch to Apache Lucene 10.4 (elastic#141882) ESQL: Add configurable bracket-based multi-value support for CSV reader (elastic#143890) time series es819 binary dv use up to a 1mb block size (elastic#143049) Dynamically enable / disable plugins in correspondence to stateless mode. (elastic#142147) ES|QL: Implement first/last_over_time for tdigest (elastic#143832) Document CHANGE_POINT limitation (elastic#143877) Fix OperationsOnSeqNoDisabledIndicesIT (elastic#143892) [Test] Test that sequence numbers are not pruned with retention lease (elastic#143825) ...
3 participants
Replaces the limited serialisation tests inside
CoordinationDiagnosticsServiceTestswithCoordinationDiagnosticsDetailsWireSerializingTests,CoordinationDiagnosticsResultWireSerializingTestsandCoordinationDiagnosticsStatusWireSerializingTests, as well as adding extra unit tests toCoordinationDiagnosticsServiceTests