Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes IndiceOptionsTests to serialise correctly #30644

Merged
merged 2 commits into from
May 16, 2018
Merged

Fixes IndiceOptionsTests to serialise correctly #30644

merged 2 commits into from
May 16, 2018

Conversation

colings86
Copy link
Contributor

Previous to this change IndicesOptionsTests.testSerialisation() would
select a complete random version for both the StreamOutput and the
StreamInput. This meant that the output could be selected as 7.0+
while the input was selected as <7.0 causing the stream to be written
in the new format and read in teh old format (or vica versa). This
change splits the two cases into different test methods ensuring that
the Streams are at least on compatibile versions even if they are on
different versions.

Previous to this change `IndicesOptionsTests.testSerialisation()` would
select a complete random version for both the `StreamOutput` and the
`StreamInput`. This meant that the output could be selected as 7.0+
while the input was selected as <7.0 causing the stream to be written
in the new format and read in teh old format (or vica versa). This
change splits the two cases into different test methods ensuring that
the Streams are at least on compatibile versions even if they are on
different versions.
@colings86 colings86 added >non-issue review :Core/Infra/Core Core issues without another label labels May 16, 2018
@colings86 colings86 self-assigned this May 16, 2018
@colings86 colings86 requested review from dakrone and jasontedor May 16, 2018 08:27
@jpountz
Copy link
Contributor

jpountz commented May 16, 2018

I'm +1 on splitting into 2 tests, but why don't we set the same version on the input and on the output?

@colings86
Copy link
Contributor Author

Good point, it actually should be the same version for both the input and output. I'll make the change

server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTest
s.java
Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

bleskes added a commit that referenced this pull request May 16, 2018
bleskes added a commit that referenced this pull request May 16, 2018
@colings86 colings86 merged commit 95ad9ab into elastic:master May 16, 2018
dakrone pushed a commit that referenced this pull request May 16, 2018
* Fixes IndiceOptionsTests to serialise correctly

Previous to this change `IndicesOptionsTests.testSerialisation()` would
select a complete random version for both the `StreamOutput` and the
`StreamInput`. This meant that the output could be selected as 7.0+
while the input was selected as <7.0 causing the stream to be written
in the new format and read in teh old format (or vica versa). This
change splits the two cases into different test methods ensuring that
the Streams are at least on compatibile versions even if they are on
different versions.

* Use same random version for input and output streams
server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTest
s.java
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 16, 2018
…ngs-to-true

* elastic/master: (34 commits)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (elastic#30411)
  Remove unused DirectoryUtils class. (elastic#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (elastic#30621)
  Fix bug in BucketMetrics path traversal (elastic#30632)
  Fixes IndiceOptionsTests to serialise correctly (elastic#30644)
  S3 repo plugin populate SettingsFilter (elastic#30652)
  mute IndicesOptionsTests.testSerialization
  Rest High Level client: Add List Tasks (elastic#29546)
  Refactors ClientHelper to combine header logic (elastic#30620)
  [ML] Wait for ML indices in rolling upgrade tests (elastic#30615)
  Watcher: Ensure secrets integration tests also run triggered watch (elastic#30478)
  Move allocation awareness attributes to list setting (elastic#30626)
  [Docs] Update code snippet in has-child-query.asciidoc (elastic#30510)
  Replace custom reloadable Key/TrustManager (elastic#30509)
  ...
martijnvg added a commit that referenced this pull request May 17, 2018
* es/master: (74 commits)
  Preserve REST client auth despite 401 response (#30558)
  [test] packaging: add windows boxes (#30402)
  Make xpack modules instead of a meta plugin (#30589)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (#30646)
  Reindex: Fixed typo in assertion failure message (#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  [DOCS] Reorganizes RBAC documentation
  SQL: Remove dependency for server's version from JDBC driver (#30631)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (#30411)
  Remove unused DirectoryUtils class. (#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (#30621)
  Fix bug in BucketMetrics path traversal (#30632)
  Fixes IndiceOptionsTests to serialise correctly (#30644)
  ...
martijnvg added a commit that referenced this pull request May 17, 2018
* es/6.x: (44 commits)
  SQL: Remove dependency for server's version from JDBC driver (#30631)
  Make xpack modules instead of a meta plugin (#30589)
  Security: Remove SecurityLifecycleService (#30526)
  Build: Add task interdependencies for ssl configuration (#30633)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (#30646)
  Reindex: Fixed typo in assertion failure message (#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  Use readFully() to read bytes from CipherInputStream (#30640)
  Add Create Repository High Level REST API (#30501)
  [DOCS] Reorganizes RBAC documentation
  Test: increase search logging for LicensingTests
  Delay _uid field data deprecation warning (#30651)
  Deprecate Empty Templates (#30194)
  Remove unused DirectoryUtils class. (#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (#30534)
  [TEST] Remove AwaitsFix in IndicesOptionsTests#testSerialization
  S3 repo plugin populates SettingsFilter (#30652)
  Rest High Level client: Add List Tasks (#29546)
  Fixes IndiceOptionsTests to serialise correctly (#30644)
  ...
martijnvg added a commit to martijnvg/elasticsearch that referenced this pull request May 17, 2018
* es/ccr: (75 commits)
  Preserve REST client auth despite 401 response (elastic#30558)
  [test] packaging: add windows boxes (elastic#30402)
  Make xpack modules instead of a meta plugin (elastic#30589)
  Mute ShrinkIndexIT
  [ML] DeleteExpiredDataAction should use client with origin (elastic#30646)
  Reindex: Fixed typo in assertion failure message (elastic#30619)
  [DOCS] Fixes list of unconverted snippets in build.gradle
  [DOCS] Reorganizes RBAC documentation
  SQL: Remove dependency for server's version from JDBC driver (elastic#30631)
  Test: increase search logging for LicensingTests
  Adjust serialization version in IndicesOptions
  [TEST] Fix compilation
  Remove version argument in RangeFieldType (elastic#30411)
  Remove unused DirectoryUtils class. (elastic#30582)
  Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534)
  Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594)
  Removes AwaitsFix on IndicesOptionsTests
  Template upgrades should happen in a system context (elastic#30621)
  Fix bug in BucketMetrics path traversal (elastic#30632)
  Fixes IndiceOptionsTests to serialise correctly (elastic#30644)
  ...
ywelsch pushed a commit to ywelsch/elasticsearch that referenced this pull request May 23, 2018
ywelsch pushed a commit to ywelsch/elasticsearch that referenced this pull request May 23, 2018
* Fixes IndiceOptionsTests to serialise correctly

Previous to this change `IndicesOptionsTests.testSerialisation()` would
select a complete random version for both the `StreamOutput` and the
`StreamInput`. This meant that the output could be selected as 7.0+
while the input was selected as <7.0 causing the stream to be written
in the new format and read in teh old format (or vica versa). This
change splits the two cases into different test methods ensuring that
the Streams are at least on compatibile versions even if they are on
different versions.

* Use same random version for input and output streams
server/src/test/java/org/elasticsearch/action/support/IndicesOptionsTest
s.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >non-issue v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants