[GitHub] metron issue #1009: METRON-1549: Add empty object test to WriterBoltIntegrat...
Github user mmiklavc commented on the issue: https://github.com/apache/metron/pull/1009 @nickwallen and @ottobackwards - I extracted the common setup components into one parameterized method. I opted for setting private test class fields with the components rather than invoking getComponent() because I thought it was a bit cleaner. I also looked at doing this in @Before and @After setup methods, but setup for each of the tests changes on a per-test basis. We're currently employing a useful builder pattern to configure the test components/infrastructure which generally means you provide invariants and then build the object from them. It might also be worthwhile to look at a way to use the builder pattern, but also change certain aspects of the setup within the individual test cases, e.g. upload a new global config after spinning up the components. This could make it more efficient to run multiple tests by using @Before and @After to spin things up to run a battery of similar integration tests. Do you guys think we need it for this test change? ---
[GitHub] metron issue #1009: METRON-1549: Add empty object test to WriterBoltIntegrat...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1009 I think this is good. My work on the builder/Processor stuff was a decent attempt to reduce duplicate code in the integration tests without a total rethink. I think what we see across the integration tests is so much commonality that it is apparent that some re-think could be useful to have more standardize test classes and use cases structured such that there is less overhead of duplicated code required. As I have done some work on other projects and seen other approaches this has only become more clear. commons-vfs for example runs the same tests against every provider, with each provider providing specialization of it's suites. It would seem that we should be able to have a more generic suite or testing harness for *n* topologies/topics working together, as a next step refactoring of my prior `Process` work. I don't think this is the PR for that though ---
[GitHub] metron issue #1009: METRON-1549: Add empty object test to WriterBoltIntegrat...
Github user nickwallen commented on the issue: https://github.com/apache/metron/pull/1009 +1 This looks good. Thanks! ---
[GitHub] metron issue #1009: METRON-1549: Add empty object test to WriterBoltIntegrat...
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/1009 +1 ---
