METRON-2168 Elasticsearch Updates Not Tested in Integration Test

[nickwallen]

The ElasticsearchUpdateIntegrationTest is not testing that the ElasticsearchDao can update and retrieve values in a manner similar to what would occur in production.

What?

Within the Elasticsearch index, the test fails to define the 'guid' field to be of type 'keyword', instead the type is defaulted to 'text'. In a production setting this mistake would prevent any documents from being found by guid. Unfortunately, the test passes despite this. The test needs to match the behavior of what a user would experience in production.

Why?

These problems arise because of the way the test is setup. Instead of directly testing an ElasticsearchDao as you might expect this test runs against a MultiIndexDao initialized with both an ElasticseachDao and an HBaseDao. On retrievals the MultIndexDao will return the document from whichever index responds first.

With the current test setup, the underlying ElasticsearchDao will never retrieve the document that the test case is expecting. In all cases where the test passes, the document is actually being returned from the HBaseDao which is actually just interacting with a mock backend. The test needs to actually test that we can update and retrieve documents from Elasticsearch.

Proof?

If you alter the test to run against just an ElasticsearchDao the test will fail as follows in the attached log file.

Testing

This only changes the integration tests. Run the integration tests to validate these changes.

Pull Request Checklist

• Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
• Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
• Has your PR been rebased against the latest commit within the target branch (typically master)?
• Have you included steps to reproduce the behavior or problem that is being changed or addressed?
• Have you included steps or a guide to how the change may be verified and tested manually?
• Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
• Have you written or updated unit tests and or integration tests to verify your changes?
• If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
• Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?

[nickwallen]

One point that reviewers should consider is do we have enough test coverage for the MultiIndexDao now?

I don't think the MultiIndexDao warrants an integration test. It doesn't integrate with any external systems. I think it can be tested sufficiently with unit testing. But is what we have in MultiIndexDaoTest enough?

update: My answer would lean on the "no" side, but I don't think the integration tests really covered any of the existing gaps. I might spend a bit of time expanding on MultiIndexDaoTest as part of this PR.

[nickwallen]

I added more unit tests for MultiIndexDao.

[mmiklavc]

One question on the guava version change.

[mmiklavc]

Is this safe to change? ie do we have an hbase enrichment running to verify the classpath doesn't have problems?

[nickwallen]

It looks like I don't need that change for this PR, so I reverted it. I may need to revisit this in #1454.

[mmiklavc]

lgtm, +1