Skip to content

Conversation

@tlrx
Copy link
Member

@tlrx tlrx commented Jun 19, 2017

Following #25257 (comment) I created a simple unit test for the XContentParserUtilsTests.parseStoredFieldsValue() method. It looks like it does not need to handle null values (because they are not printed out by MappedFieldType).

@tlrx tlrx added :Core/Infra/Core Core issues without another label review >test Issues or PRs that are addressing/adding tests v5.6.0 v6.0.0 labels Jun 19, 2017
@tlrx tlrx requested a review from cbuescher June 19, 2017 11:30
Copy link
Member

@javanna javanna 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 @tlrx

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM, left one very minor nit, feel free to merge right away if you like.

}

public void testParseStoredFieldsValueInt() throws IOException {
final Integer value = randomInt(1_000);
Copy link
Member

Choose a reason for hiding this comment

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

nit: can we test the whole positive/negative int range here? I think it shouldn't complicate things a lot and since its testing the parser it would provide better coverage.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, thanks!

@tlrx tlrx merged commit 6a792d6 into elastic:master Jun 23, 2017
@tlrx
Copy link
Member Author

tlrx commented Jun 23, 2017

Thanks @javanna @cbuescher

@tlrx tlrx deleted the add-test-parseStoredFieldsValue branch June 23, 2017 09:37
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 23, 2017
* master:
  testCreateShrinkIndex: removed left over debugging log line that violated linting
  testCreateShrinkIndex should make sure to use the right source stats when testing shrunk target
  [Test] Add unit test for XContentParserUtilsTests.parseStoredFieldsValue (elastic#25288)
  Update percolate-query.asciidoc (elastic#25364)
  Remove remaining `index.mapping.single_type=false` (elastic#25369)
  test: Replace OldIndexBackwardsCompatibilityIT#testOldClusterStates with a full cluster restart qa test
  Fix use of spaces on Windows if JAVA_HOME not set
  ESIndexLevelReplicationTestCase.ReplicationAction#execute should send exceptions to it's listener rather than bubble them up
  testRecoveryAfterPrimaryPromotion shouldn't flush the replica with extra operations
  fix sort and string processor tests around targetField (elastic#25358)
  Ensure `InternalEngineTests.testConcurrentWritesAndCommits` doesn't pile up commits (elastic#25367)
  [TEST] Add debug logging if an unexpected exception is thrown
  Update Painless to Allow Augmentation from Any Class (elastic#25360)
  TemplateUpgraders should be called during rolling restart (elastic#25263)
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 >test Issues or PRs that are addressing/adding tests v5.6.0 v6.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants