Generalize scap parsing#15
Generalize scap parsing#15akofink merged 2 commits intoOpenSCAP:masterfrom akofink:generalize-scap-parsing
Conversation
| } | ||
| @report_parser = ::OpenscapParser::Base.new(fake_report) | ||
| assert_equal 1, @report_parser.profiles.count | ||
| assert_equal 1, @report_parser.test_result_profiles.count |
There was a problem hiding this comment.
If you want profiles associated with the test result only, you've got to ask for it. :)
|
|
||
| def xpath_nodes(xpath) | ||
| @parsed_xml.xpath(xpath) | ||
| end |
There was a problem hiding this comment.
These are nice helpers. We could do the same for css_node and css_nodes, but we should be able to do everything with xpath.
|
Notice the two commits to ease reviewing |
|
|
||
| def end_time | ||
| @end_time ||= DateTime.parse(test_result_node['end-time']) | ||
| end |
There was a problem hiding this comment.
Moved to TestResult, as they come from that node
| class Profile | ||
| attr_acessor :id, :title, :description | ||
| def initialize(profile_xml: nil) | ||
| @profile_xml = profile_xml |
There was a problem hiding this comment.
I like this architecture of passing in the Nokogiri::Element like we do for OpenscapParser::Rule objects
| # Methods related to saving profiles and finding which hosts | ||
| # they belong to | ||
| module Profiles | ||
| include TestResult |
There was a problem hiding this comment.
No need for these mixins to be coupled so tightly IMO
| base.class_eval do | ||
| def profiles | ||
| @profiles ||= { | ||
| profile_node['id'] => profile_node.at_css('title').text |
There was a problem hiding this comment.
This method is now test_result_profiles from the TestResult module. It returns the list of (one) profiles that this scan was run against.
| @profiles ||= profile_nodes.inject({}) do |profiles, profile_node| | ||
| profiles[profile_node['id']] = profile_node.at_css('title').text | ||
|
|
||
| private |
There was a problem hiding this comment.
Nothing in here needs to be private IMO. It's useful to retrieve the raw Nokogiri::NodeSet or an array of OpenscapParser::Profile objects.
|
Looking good. It will need a rebase and replacing safe navigation operator to be compatible with older version of ruby. |
We're using this module for any SCAP xml content, not just reports Signed-off-by: Andrew Kofink <akofink@redhat.com>
|
Updated to remove the safe navigation operator |
Signed-off-by: Andrew Kofink <akofink@redhat.com>
| DateTime.parse(test_result_node['end-time']) | ||
| end | ||
|
|
||
| def test_result_profiles |
There was a problem hiding this comment.
@bastilian funnily enough, MiniTest::Test only has a run method, but it looks for methods which start with test_. That creates the scenario you predicted would happen! 😝 So I've had to wrap the profiles_test tests in a dummy test class like you suggested.
|
Tests are green now, and all comments addressed |
xprazak2
left a comment
xprazak2
left a comment
There was a problem hiding this comment.
Works for me, I think we can merge if there are no additional comments.
|
It's good to merge IMO - it'd be good to test it with any apps that use this gem (I've tested with compliance-backend, and it works fine). |
2 participants
We initially wrote this openscap_parser in the context of parsing SCAP scan results. Now we want to use it for parsing datastreams without results. This PR is only a refactoring to generalize some of the code and move things around to be specific to results parsing or not. Behavior should not change!