Skip to content

Conversation

@markharwood
Copy link
Contributor

We're busy deprecating like crazy with types removal and what's concerning is the number of existing LLRC/HLRC tests where we're forced to switch off all warnings checks just to make tests pass.

  • The answer is not to rewrite tests to use only new APIs because we want to test the deprecated APIs still work.
  • The answer is not to turn off strict warning checks at class level (or method invocation level) because we go blind to any unanticipated warnings.

The answer is for tests to invoke known-deprecated APIs and list the warnings they expect to receive as a result.

This PR adds a parameter in RequestOptions called "expectedWarnings" which lists expected messages. If supplied in strictDeprecationMode=true then LLRC:

  1. suppresses any exception-throwing for received warnings that were listed as expected
  2. causes an exception if listed expected warnings were not received

@markharwood markharwood added >test Issues or PRs that are addressing/adding tests :Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch :Core/Features/Java High Level REST Client labels Dec 6, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@markharwood
Copy link
Contributor Author

  1. causes an exception if listed expected warnings were not received

There's a concern that some tests can't be certain which node (and therefore which es version) handles a request - this means expected warnings may not materialise. Maybe there's a requirement for an option to relax the strictness so that rule 1 always applies (expected warnings suppress exceptions) and rule 2 is relaxed (unmet expectations are ignored)

@markharwood
Copy link
Contributor Author

Closing - superseded by #36345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Clients/Java Low Level REST Client Minimal dependencies Java Client for Elasticsearch >test Issues or PRs that are addressing/adding tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants