You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by merrimanr <gi...@git.apache.org> on 2017/05/01 22:51:13 UTC

[GitHub] incubator-metron issue #526: Metron-846: Add E2E tests for metron management...

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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---