search

OpenSCAP / openscap_parser

MIT License
7 stars 14 forks source link

Add a test for ospp42 in rhel7 SSG #11

Closed akofink closed 5 years ago

akofink commented 5 years ago

Towards https://projects.engineering.redhat.com/browse/RHICOMPL-329. It appears the bug isn't in the openscap_parser, but this test is still valid.

Signed-off-by: Andrew Kofink akofink@redhat.com

bastilian commented 5 years ago

It might not be good to mix OpenscapParser::Profilesand OpenscapParser::XMLReport into a dummy class or object, rather than into the ProfilesTest#describe. Something like so in the setup method should make it safer.

    @dummy_object = Object.new
    @dummy_object.send(:include, OpenscapParser::Profiles)
    @dummy_object.send(:include, OpenscapParser::XMLReport)

My worry is that in the future especially with method names like test_result_... one gets added that causes havoc.

akofink commented 5 years ago

This is essentially what is happening as written. When using describe, Minitest "defines a test class subclassing from either Minitest::Spec or from the surrounding describe's class."

bastilian commented 5 years ago

What could happen with including the modules to test into the Minitest::Spec or a sub-class of it, is that methods are mixed up. For example, if Minitest::Spec had a method description that it relied on, mixing in OpenscapParser::XMLReport would override that method.

Having a empty dummy class and including the modules into there makes it simply safer to test.

akofink commented 5 years ago

It's a valid point, but I don't think we have to be too concerned. MiniTest::Test has only one instance method: run. Also, I'm not changing this aspect of the code in this PR - I'm only adding to what already exists.