Skip to content

[Test] Add unit test for XContentParserUtilsTests.parseStoredFieldsValue#25288

Merged
tlrx merged 2 commits intoelastic:masterfrom
tlrx:add-test-parseStoredFieldsValue
Jun 23, 2017
Merged

[Test] Add unit test for XContentParserUtilsTests.parseStoredFieldsValue#25288
tlrx merged 2 commits intoelastic:masterfrom
tlrx:add-test-parseStoredFieldsValue

Conversation

@tlrx
Copy link
Copy Markdown
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
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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