Github user merrimanr commented on the issue:
https://github.com/apache/incubator-metron/pull/526
I am now able to get this to run and all tests passed. Really great work
so far.
I have a couple of requests. First there are some very minor style issues.
Several lines are not terminated so that should be cleaned up.
I feel like the organization of the "Sensor Config for parser e2e1" is too
broad. I would prefer tests to be broken down a little bit more, by feature or
editor pane if possible. For example, as a test I swapped the sort orders in
threat triage editor. The tests failed (yay!) but the test description
reported in the failure was "Sensor Config for parser e2e1 should add e2e
parser". That is too vague and gives me no indication of where the error
happened and what part of the code I need to inspect.
I think there may also be some gaps in testing. Is the raw JSON editor
tested? Advanced config? Are we testing how we format field names and values
in the read-only and list view? When we're editing sensors are we verifying
the correct config is saved? Are we verifying the editor looks correct when we
open an existing sensor (not just the primary form but child panes as well)?
Are we testing the general settings page?
Another concern I have is that we're expecting a static list of sensors.
These tests will break if a sensor is ever added to Metron and it won't be
obvious because these tests are not part of the build workflow. Is it
possible/reasonable to add some kind of setup script to create the test cases
so that they will not be affected by new parsers being added?
Still more work to do but this is a solid start.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
