Skip to content

Conversation

@cbuescher
Copy link
Member

Changes made in #28972 seems to have changed some assumptions about how
SMILE and CBOR write byte[] values and how this is tested. This changes
the generation of the randomized DocumentField values back to BytesArray
while expecting the JSON and YAML deserialisation to produce Base64
encoded strings and SMILE and CBOR to parse back BytesArray instances.

Closes #29080

Changes made in elastic#28972 seems to have changed some assumptions about how
SMILE and CBOR write byte[] values and how this is tested. This changes
the generation of the randomized DocumentField values back to BytesArray
while expecting the JSON and YAML deserialisation to produce Base64
encoded strings and SMILE and CBOR to parse back BytesArray instances.

Closes elastic#29080
@cbuescher cbuescher added >test Issues or PRs that are addressing/adding tests review v7.0.0 v6.3.0 labels Mar 15, 2018
@cbuescher cbuescher requested review from dakrone and tlrx March 15, 2018 11:21
@cbuescher
Copy link
Member Author

@dakrone @tlrx changes in #28972 seem to have caused test failures, I changed the creation of the random values for the DocumentFields back since I think the intention of the original test was to see how byte array field values are parsed back. I'm not sure if the tests should be changed in a different way after the changes made in #28972 though. Could you both take a look please?

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me.

Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this

@cbuescher cbuescher merged commit 312ccc0 into elastic:master Mar 15, 2018
@cbuescher
Copy link
Member Author

thanks for the reviews

cbuescher pushed a commit that referenced this pull request Mar 15, 2018
Changes made in #28972 seems to have changed some assumptions about how
SMILE and CBOR write byte[] values and how this is tested. This changes
the generation of the randomized DocumentField values back to BytesArray
while expecting the JSON and YAML deserialisation to produce Base64
encoded strings and SMILE and CBOR to parse back BytesArray instances.

Closes #29080
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/6.x: (89 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  [Docs] Fix link to Grok patterns (#29088)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  Fix Parsing Bug with Update By Query for Stored Scripts (#29039)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  [Docs] Fix Java Api index administration usage (#28260)
  Improve error message for installing plugin (#28298)
  Decouple XContentBuilder from BytesReference (#28972)
  ...
martijnvg added a commit that referenced this pull request Mar 16, 2018
* es/master: (97 commits)
  Clarify requirements of strict date formats. (#29090)
  Clarify that dates are always rendered as strings. (#29093)
  Compilation fix for #29067
  [Docs] Fix link to Grok patterns (#29088)
  Store offsets in index prefix fields when stored in the parent field (#29067)
  Fix starting on Windows from another drive (#29086)
  Use removeTask instead of finishTask in PersistentTasksClusterService (#29055)
  Added minimal docs for reindex api in java-api docs
  Allow overriding JVM options in Windows service (#29044)
  Clarify how to set compiler and runtime JDKs (#29101)
  CLI: Close subcommands in MultiCommand (#28954)
  TEST: write ops should execute under shard permit (#28966)
  [DOCS] Add X-Pack upgrade details (#29038)
  Revert "Improve error message for installing plugin (#28298)"
  Docs: HighLevelRestClient#exists (#29073)
  Validate regular expressions in dynamic templates. (#29013)
  [Tests] Fix GetResultTests and DocumentFieldTests failures (#29083)
  Reenable LiveVersionMapTests.testRamBytesUsed on Java 9. (#29063)
  Mute failing GetResultTests and DocumentFieldTests
  Improve error message for installing plugin (#28298)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>test Issues or PRs that are addressing/adding tests v6.3.0 v7.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants