[issue-788] fix tag-value parser to allow NONE and NOASSERTION for pa… #816
Conversation
meretp
force-pushed
the
noassertion-handling
branch
from
d761116
to
a81b3aa
Compare
|
based this PR on #814 to (hopefully) have a successful CI |
meretp
force-pushed
the
noassertion-handling
branch
from
475412d
to
140998d
Compare
| @@ -180,7 +180,7 @@ def p_current_element_error(self, p): | |||
| "file_comment : FILE_COMMENT text_or_line\n " | |||
| "file_license_concluded : FILE_LICENSE_CONCLUDED license_or_no_assertion_or_none\n " | |||
| "package_name : PKG_NAME LINE\n description : PKG_DESCRIPTION text_or_line\n " | |||
| "summary : PKG_SUMMARY text_or_line\n source_info : PKG_SOURCE_INFO text_or_line\n " | |||
| "summary : PKG_SUMMARY text_or_line\n source_info : PKG_SOURCE_INFO text_or_line_including_no_assertion\n " | |||
There was a problem hiding this comment.
The problem at hand can also occur for other text_or_line fields apart from source_info. Basically all properties that use text_or_line. So I would believe that the whole text_or_line property has to be adapted, not only for the source information case.
There was a problem hiding this comment.
Also, the same problem will arise when somebody uses NONE. So best solve this one here, too.
There was a problem hiding this comment.
Right, I changed the behaviour such that text_or_line parses NOASSERTION and NONE as regular strings and for fields that allow NOASSERTION or NONE, like the license fields these strings will be serialized as the corresponding SpdxNoAssertion and SpdxNone objects
meretp
force-pushed
the
noassertion-handling
branch
from
2423d41
to
422e6e0
Compare
| @@ -83,6 +83,26 @@ def test_parse_package(): | |||
| assert package.valid_until_date == datetime(2022, 1, 1, 12) | |||
|
|
|||
|
|
|||
| def test_parse_package_with_no_assertion_as_source_info(): | |||
There was a problem hiding this comment.
Ideally, this test should include all fields that can sport text only, all filled with either NONE or NOASSERTION. Might be bit overkill, though.
However, at least a field filled with NONE should be present, I think.
There was a problem hiding this comment.
Adding all cases would be overkill imho but I added a field that contains NONE and is parsed as text.
| @@ -83,6 +83,26 @@ def test_parse_package(): | |||
| assert package.valid_until_date == datetime(2022, 1, 1, 12) | |||
|
|
|||
|
|
|||
| def test_parse_package_with_no_assertion_as_source_info(): | |||
There was a problem hiding this comment.
We might want to rename this to test_parse_none_and_no_assertion_as_text or something. Otherwise we emphasize the source info despite it not being a special case.
There was a problem hiding this comment.
I agree, I also moved the test to the test_tag_valu_parser.pymodule as it is not specific to packages.
meretp
force-pushed
the
noassertion-handling
branch
2 times, most recently
from
43fde85
to
1ab91dd
Compare
…ckage source info as they are valid strings Signed-off-by: Meret Behrens <[email protected]> # fixes 788
Signed-off-by: Meret Behrens <[email protected]>
…ckage source info as they are valid strings Signed-off-by: Meret Behrens <[email protected]>
Signed-off-by: Meret Behrens <[email protected]>
meretp
force-pushed
the
noassertion-handling
branch
from
1ab91dd
to
cd676e3
Compare
|
@meretp & @armintaenzertng : can this be merged? |
…ckage source info as they are valid strings
fixes 788
Signed-off-by: Meret Behrens [email protected]