From f164353f8f9627788334c8420b82fc0b02ccedd0 Mon Sep 17 00:00:00 2001 From: yash-nisar Date: Thu, 15 Mar 2018 22:52:09 +0530 Subject: [PATCH] Return messages instead of modifying list #50 We now return messages from each `validation` function instead of modifying lists in place. It is preferrable to have side-effect free functions, so in this case, return a new list with the element modified. There are a few rare corner use cases where in place changes can make sense, but 99% of the times, it a bad idea and can trigger any kind of weird, byzantine and hard to debug bugs. Signed-off-by: Yash Nisar --- spdx/creationinfo.py | 34 ++--- spdx/document.py | 102 ++++++--------- spdx/file.py | 128 +++++++++--------- spdx/package.py | 150 ++++++++++------------ spdx/parsers/rdf.py | 10 +- spdx/parsers/tagvalue.py | 10 +- spdx/review.py | 43 +++---- spdx/writers/rdf.py | 4 +- spdx/writers/tagvalue.py | 3 +- tests/data/doc_write/rdf-simple-plus.json | 8 +- tests/data/doc_write/rdf-simple.json | 8 +- tests/data/doc_write/tv-simple-plus.tv | 1 + tests/data/doc_write/tv-simple.tv | 1 + tests/test_conversion.py | 16 +-- tests/test_document.py | 15 ++- 15 files changed, 245 insertions(+), 288 deletions(-) diff --git a/spdx/creationinfo.py b/spdx/creationinfo.py index 628bd49c0..c29b6cf11 100644 --- a/spdx/creationinfo.py +++ b/spdx/creationinfo.py @@ -152,32 +152,24 @@ def created_iso_format(self): def has_comment(self): return self.comment is not None - def validate(self, messages=None): + def validate(self, messages): """Returns True if the fields are valid according to the SPDX standard. Appends user friendly messages to the messages parameter. """ - # FIXME: messages should be returned - messages = messages if messages is not None else [] + messages = self.validate_creators(messages) + messages = self.validate_created(messages) - return (self.validate_creators(messages) - and self.validate_created(messages)) + return messages - def validate_creators(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + def validate_creators(self, messages): + if len(self.creators) == 0: + messages = messages + [ + 'No creators defined, must have at least one.'] - if len(self.creators) != 0: - return True - else: - messages.append('No creators defined, must have at least one.') - return False + return messages - def validate_created(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + def validate_created(self, messages): + if self.created is None: + messages = messages + ['Creation info missing created date.'] - if self.created is not None: - return True - else: - messages.append('Creation info missing created date.') - return False + return messages diff --git a/spdx/document.py b/spdx/document.py index 7d307fc84..42040ad98 100644 --- a/spdx/document.py +++ b/spdx/document.py @@ -173,15 +173,11 @@ def __lt__(self, other): def add_xref(self, ref): self.cross_ref.append(ref) - def validate(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] - + def validate(self, messages): if self.text is None: - messages.append('ExtractedLicense text can not be None') - return False - else: - return True + messages = messages + ['ExtractedLicense text can not be None'] + + return messages class Document(object): @@ -227,87 +223,65 @@ def files(self, value): def has_comment(self): return self.comment is not None - def validate(self, messages=None): + def validate(self, messages): """ Validate all fields of the document and update the messages list with user friendly error messages for display. """ - # FIXME: messages should be returned - messages = messages if messages is not None else [] + messages = self.validate_version(messages) + messages = self.validate_data_lics(messages) + messages = self.validate_creation_info(messages) + messages = self.validate_package(messages) + messages = self.validate_extracted_licenses(messages) + messages = self.validate_reviews(messages) - return (self.validate_version(messages) - and self.validate_data_lics(messages) - and self.validate_creation_info(messages) - and self.validate_package(messages) - and self.validate_extracted_licenses(messages) - and self.validate_reviews(messages) - ) - - def validate_version(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages + def validate_version(self, messages): if self.version is None: - messages.append('Document has no version.') - return False - else: - return True + messages = messages + ['Document has no version.'] - def validate_data_lics(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages + def validate_data_lics(self, messages): if self.data_license is None: - messages.append('Document has no data license.') - return False + messages = messages + ['Document has no data license.'] - if self.data_license.identifier == 'CC0-1.0': - return True - else: - # FIXME: REALLY? what if someone wants to use something else? - messages.append('Document data license must be CC0-1.0.') - return False + # FIXME: REALLY? what if someone wants to use something else? + if self.data_license.identifier != 'CC0-1.0': + messages = messages + ['Document data license must be CC0-1.0.'] - def validate_reviews(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages - valid = True + def validate_reviews(self, messages): for review in self.reviews: - valid = review.validate(messages) and valid - return valid + messages = review.validate(messages) - def validate_creation_info(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages + def validate_creation_info(self, messages): if self.creation_info is not None: - return self.creation_info.validate(messages) + messages = self.creation_info.validate(messages) else: - messages.append('Document has no creation information.') - return False + messages = messages + ['Document has no creation information.'] - def validate_package(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages + def validate_package(self, messages): if self.package is not None: - return self.package.validate(messages) + messages = self.package.validate(messages) else: - messages.append('Document has no package.') - return False + messages = messages + ['Document has no package.'] - def validate_extracted_licenses(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages - valid = True + def validate_extracted_licenses(self, messages): for lic in self.extracted_licenses: if isinstance(lic, ExtractedLicense): - valid = lic.validate(messages) and valid + messages = lic.validate(messages) else: - messages.append( + messages = messages + [ 'Document extracted licenses must be of type ' - 'spdx.document.ExtractedLicense and not ' + type(lic)) - valid = False - return valid + 'spdx.document.ExtractedLicense and not ' + type(lic) + ] + return messages diff --git a/spdx/file.py b/spdx/file.py index 868207125..43fe18bb1 100644 --- a/spdx/file.py +++ b/spdx/file.py @@ -93,90 +93,82 @@ def add_artifact(self, symbol, value): artifact = getattr(self, symbol) artifact.append(value) - def validate(self, messages=None): + def validate(self, messages): """Validates the fields and appends user friendly messages to messages parameter if there are errors. """ - # FIXME: messages should be returned - messages = messages if messages is not None else [] + messages = self.validate_concluded_license(messages) + messages = self.validate_type(messages) + messages = self.validate_chksum(messages) + messages = self.validate_licenses_in_file(messages) + messages = self.validate_copyright(messages) + messages = self.validate_artifacts(messages) - return (self.validate_concluded_license(messages) - and self.validate_type(messages) - and self.validate_chksum(messages) - and self.validate_licenses_in_file(messages) - and self.validate_copyright(messages) - and self.validate_artifacts(messages)) + return messages - def validate_copyright(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] - - if isinstance( + def validate_copyright(self, messages): + if not isinstance( self.copyright, - (six.string_types, six.text_type, utils.NoAssert, utils.SPDXNone)): - return True - else: - messages.append('File copyright must be str or unicode or utils.NoAssert or utils.SPDXNone') - return False + (six.string_types, six.text_type, utils.NoAssert, utils.SPDXNone) + ): + messages = messages + [ + 'File copyright must be str or unicode or ' + 'utils.NoAssert or utils.SPDXNone' + ] - def validate_artifacts(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages - if (len(self.artifact_of_project_home) >= - max(len(self.artifact_of_project_uri), len(self.artifact_of_project_name))): - return True - else: - messages.append('File must have as much artifact of project as uri or homepage') - return False + def validate_artifacts(self, messages): + if (len(self.artifact_of_project_home) < + max(len(self.artifact_of_project_uri), + len(self.artifact_of_project_name))): + messages = messages + [ + 'File must have as much artifact of project as uri or homepage'] - def validate_licenses_in_file(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages + def validate_licenses_in_file(self, messages): # FIXME: what are we testing the length of a list? or? if len(self.licenses_in_file) == 0: - messages.append('File must have at least one license in file.') - return False - else: - return True + messages = messages + [ + 'File must have at least one license in file.' + ] - def validate_concluded_license(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] + return messages - if isinstance(self.conc_lics, (document.License, utils.NoAssert, - utils.SPDXNone)): - return True - else: - messages.append( + def validate_concluded_license(self, messages): + # FIXME: use isinstance instead?? + if not isinstance(self.conc_lics, (document.License, utils.NoAssert, + utils.SPDXNone)): + messages = messages + [ 'File concluded license must be one of ' - 'document.License, utils.NoAssert or utils.SPDXNone') - return False - - def validate_type(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] - - if self.type in [None, FileType.SOURCE, FileType.OTHER, FileType.BINARY, FileType.ARCHIVE]: - return True + 'document.License, utils.NoAssert or utils.SPDXNone' + ] + + return messages + + def validate_type(self, messages): + if self.type not in [ + None, FileType.SOURCE, FileType.OTHER, FileType.BINARY, + FileType.ARCHIVE + ]: + messages = messages + [ + 'File type must be one of the constants defined in ' + 'class spdx.file.FileType' + ] + + return messages + + def validate_chksum(self, messages): + if not isinstance(self.chk_sum, checksum.Algorithm): + messages = messages + [ + 'File checksum must be instance of spdx.checksum.Algorithm' + ] else: - messages.append('File type must be one of the constants defined in class spdx.file.FileType') - return False - - def validate_chksum(self, messages=None): - # FIXME: messages should be returned - messages = messages if messages is not None else [] - - if isinstance(self.chk_sum, checksum.Algorithm): - if self.chk_sum.identifier == 'SHA1': - return True - else: - messages.append('File checksum algorithm must be SHA1') - return False - else: - messages.append('File checksum must be instance of spdx.checksum.Algorithm') - return False + if not self.chk_sum.identifier == 'SHA1': + messages = messages + ['File checksum algorithm must be SHA1'] + + return messages def calc_chksum(self): BUFFER_SIZE = 65536 diff --git a/spdx/package.py b/spdx/package.py index d67391fdf..81c21bdb5 100644 --- a/spdx/package.py +++ b/spdx/package.py @@ -83,93 +83,81 @@ def add_lics_from_file(self, lics): def add_exc_file(self, filename): self.verif_exc_files.append(filename) - def validate(self, messages=None): + def validate(self, messages): """ Validate the package fields. Append user friendly error messages to the `messages` list. """ - # FIXME: we should return messages instead - messages = messages if messages is not None else [] + messages = self.validate_checksum(messages) + messages = self.validate_optional_str_fields(messages) + messages = self.validate_mandatory_str_fields(messages) + messages = self.validate_files(messages) + messages = self.validate_mandatory_fields(messages) + messages = self.validate_optional_fields(messages) - return (self.validate_checksum(messages) - and self.validate_optional_str_fields(messages) - and self.validate_mandatory_str_fields(messages) - and self.validate_files(messages) - and self.validate_mandatory_fields(messages) - and self.validate_optional_fields(messages)) - - def validate_optional_fields(self, messages=None): - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - - valid = True + return messages + def validate_optional_fields(self, messages): if self.originator and not isinstance(self.originator, (utils.NoAssert, creationinfo.Creator)): - messages.append( + messages = messages + [ 'Package originator must be instance of ' - 'spdx.utils.NoAssert or spdx.creationinfo.Creator') - valid = False + 'spdx.utils.NoAssert or spdx.creationinfo.Creator' + ] if self.supplier and not isinstance(self.supplier, (utils.NoAssert, creationinfo.Creator)): - messages.append( + messages = messages + [ 'Package supplier must be instance of ' - 'spdx.utils.NoAssert or spdx.creationinfo.Creator') - valid = False - - return valid - - def validate_mandatory_fields(self, messages=None): - # FIXME: we should return messages instead - messages = messages if messages is not None else [] + 'spdx.utils.NoAssert or spdx.creationinfo.Creator' + ] - valid = True + return messages + def validate_mandatory_fields(self, messages): if not isinstance(self.conc_lics, (utils.SPDXNone, utils.NoAssert, document.License)): - messages.append( + messages = messages + [ 'Package concluded license must be instance of ' - 'spdx.utils.SPDXNone or spdx.utils.NoAssert or spdx.document.License') - valid = False + 'spdx.utils.SPDXNone or spdx.utils.NoAssert or ' + 'spdx.document.License' + ] if not isinstance(self.license_declared, (utils.SPDXNone, utils.NoAssert, document.License)): - messages.append( + messages = messages + [ 'Package declared license must be instance of ' - 'spdx.utils.SPDXNone or spdx.utils.NoAssert or spdx.document.License') - valid = False + 'spdx.utils.SPDXNone or spdx.utils.NoAssert or ' + 'spdx.document.License' + ] # FIXME: this is obscure and unreadable license_from_file_check = lambda prev, el: prev and isinstance(el, (document.License, utils.SPDXNone, utils.NoAssert)) if not reduce(license_from_file_check, self.licenses_from_files, True): - messages.append( + messages = messages + [ 'Each element in licenses_from_files must be instance of ' - 'spdx.utils.SPDXNone or spdx.utils.NoAssert or spdx.document.License') - valid = False + 'spdx.utils.SPDXNone or spdx.utils.NoAssert or ' + 'spdx.document.License' + ] if not self.licenses_from_files: - messages.append('Package licenses_from_files can not be empty') - valid = False + messages = messages + [ + 'Package licenses_from_files can not be empty' + ] - return valid - - def validate_files(self, messages=None): - # FIXME: we should return messages instead - messages = messages if messages is not None else [] + return messages + def validate_files(self, messages): if not self.files: - messages.append('Package must have at least one file.') - return False + messages = messages + [ + 'Package must have at least one file.' + ] else: - valid = True for f in self.files: - valid = f.validate(messages) and valid - return valid + messages = f.validate(messages) + + return messages - def validate_optional_str_fields(self, messages=None): + def validate_optional_str_fields(self, messages): """Fields marked as optional and of type string in class docstring must be of a type that provides __str__ method. """ - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - FIELDS = [ 'file_name', 'version', @@ -178,57 +166,49 @@ def validate_optional_str_fields(self, messages=None): 'summary', 'description' ] - return self.validate_str_fields(FIELDS, True, messages) + messages = self.validate_str_fields(FIELDS, True, messages) + + return messages - def validate_mandatory_str_fields(self, messages=None): + def validate_mandatory_str_fields(self, messages): """Fields marked as Mandatory and of type string in class docstring must be of a type that provides __str__ method. """ - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - FIELDS = ['name', 'download_location', 'verif_code', 'cr_text'] - return self.validate_str_fields(FIELDS, False, messages) + messages = self.validate_str_fields(FIELDS, False, messages) + + return messages - def validate_str_fields(self, fields, optional, messages=None): + def validate_str_fields(self, fields, optional, messages): """Helper for validate_mandatory_str_field and validate_optional_str_fields""" - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - - valid = True for field_str in fields: field = getattr(self, field_str) if field is not None: # FIXME: this does not make sense??? attr = getattr(field, '__str__', None) if not callable(attr): - messages.append( - '{0} must provide __str__ method.'.format(field)) + messages = messages + [ + '{0} must provide __str__ method.'.format(field) + ] # Continue checking. - valid = False elif not optional: - messages.append('Package {0} can not be None.'.format(field_str)) - valid = False - - return valid + messages = messages + [ + 'Package {0} can not be None.'.format(field_str) + ] - def validate_checksum(self, messages=None): - # FIXME: we should return messages instead - messages = messages if messages is not None else [] + return messages - if self.check_sum is None: - return True - - if isinstance(self.check_sum, checksum.Algorithm): - if self.check_sum.identifier == 'SHA1': - return True - else: - messages.append('File checksum algorithm must be SHA1') - return False + def validate_checksum(self, messages): + if not isinstance(self.check_sum, checksum.Algorithm): + messages = messages + [ + 'Package checksum must be instance of spdx.checksum.Algorithm' + ] else: - messages.append('Package checksum must be instance of spdx.checksum.Algorithm') - return False + if self.check_sum.identifier != 'SHA1': + messages = messages + ['File checksum algorithm must be SHA1'] + + return messages def calc_verif_code(self): hashes = [] diff --git a/spdx/parsers/rdf.py b/spdx/parsers/rdf.py index 7da300990..12e0fad5b 100644 --- a/spdx/parsers/rdf.py +++ b/spdx/parsers/rdf.py @@ -757,10 +757,12 @@ def parse(self, fil): validation_messages = [] # Report extra errors if self.error is False otherwise there will be # redundent messages - if (not self.error) and (not self.doc.validate(validation_messages)): - for msg in validation_messages: - self.logger.log(msg) - self.error = True + validation_messages = self.doc.validate(validation_messages) + if not self.error: + if validation_messages: + for msg in validation_messages: + self.logger.log(msg) + self.error = True return self.doc, self.error def parse_creation_info(self, ci_term): diff --git a/spdx/parsers/tagvalue.py b/spdx/parsers/tagvalue.py index b9ae066d1..87fed20ba 100644 --- a/spdx/parsers/tagvalue.py +++ b/spdx/parsers/tagvalue.py @@ -1231,8 +1231,10 @@ def parse(self, text): validation_messages = [] # Report extra errors if self.error is False otherwise there will be # redundent messages - if (not self.error) and (not self.document.validate(validation_messages)): - for msg in validation_messages: - self.logger.log(msg) - self.error = True + validation_messages = self.document.validate(validation_messages) + if not self.error: + if validation_messages: + for msg in validation_messages: + self.logger.log(msg) + self.error = True return self.document, self.error diff --git a/spdx/review.py b/spdx/review.py index 186b31dc2..ae7a1b597 100644 --- a/spdx/review.py +++ b/spdx/review.py @@ -61,32 +61,23 @@ def review_date_iso_format(self): def has_comment(self): return self.comment is not None - def validate(self, messages=None): + def validate(self, messages): """Returns True if all the fields are valid. Appends any error messages to messages parameter. """ - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - - return (self.validate_reviewer(messages) - and self.validate_review_date(messages)) - - def validate_reviewer(self, messages=None): - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - - if self.reviewer is not None: - return True - else: - messages.append('Review missing reviewer.') - return False - - def validate_review_date(self, messages=None): - # FIXME: we should return messages instead - messages = messages if messages is not None else [] - - if self.review_date is not None: - return True - else: - messages.append('Review missing review date.') - return False + messages = self.validate_reviewer(messages) + messages = self.validate_review_date(messages) + + return messages + + def validate_reviewer(self, messages): + if self.reviewer is None: + messages = messages + ['Review missing reviewer.'] + + return messages + + def validate_review_date(self, messages): + if self.review_date is None: + messages = messages + ['Review missing review date.'] + + return messages diff --git a/spdx/writers/rdf.py b/spdx/writers/rdf.py index f6f3199bd..961277d1e 100644 --- a/spdx/writers/rdf.py +++ b/spdx/writers/rdf.py @@ -566,8 +566,8 @@ def write_document(document, out, validate=True): if validate: messages = [] - is_valid = document.validate(messages) - if not is_valid: + messages = document.validate(messages) + if messages: raise InvalidDocumentError(messages) writer = Writer(document, out) diff --git a/spdx/writers/tagvalue.py b/spdx/writers/tagvalue.py index 9af92ef1e..615b5fc06 100644 --- a/spdx/writers/tagvalue.py +++ b/spdx/writers/tagvalue.py @@ -229,7 +229,8 @@ def write_document(document, out, validate=True): InvalidDocumentError if document.validate returns False. """ messages = [] - if validate and not document.validate(messages): + messages = document.validate(messages) + if validate and messages: raise InvalidDocumentError(messages) # Write out document information diff --git a/tests/data/doc_write/rdf-simple-plus.json b/tests/data/doc_write/rdf-simple-plus.json index 1b792534a..870eb602a 100644 --- a/tests/data/doc_write/rdf-simple-plus.json +++ b/tests/data/doc_write/rdf-simple-plus.json @@ -19,7 +19,13 @@ "ns1:licenseConcluded": { "@rdf:resource": "http://spdx.org/rdf/terms#noassertion" }, - "ns1:copyrightText": "Some copyrught" + "ns1:copyrightText": "Some copyrught", + "ns1:checksum": { + "ns1:Checksum": { + "ns1:algorithm": "SHA1", + "ns1:checksumValue": "SOME-SHA1" + } + } } }, "ns1:specVersion": "SPDX-2.1", diff --git a/tests/data/doc_write/rdf-simple.json b/tests/data/doc_write/rdf-simple.json index ff436589c..4c205e8a5 100644 --- a/tests/data/doc_write/rdf-simple.json +++ b/tests/data/doc_write/rdf-simple.json @@ -19,7 +19,13 @@ "ns1:licenseConcluded": { "@rdf:resource": "http://spdx.org/rdf/terms#noassertion" }, - "ns1:copyrightText": "Some copyrught" + "ns1:copyrightText": "Some copyrught", + "ns1:checksum": { + "ns1:Checksum": { + "ns1:algorithm": "SHA1", + "ns1:checksumValue": "SOME-SHA1" + } + } } }, "ns1:specVersion": "SPDX-2.1", diff --git a/tests/data/doc_write/tv-simple-plus.tv b/tests/data/doc_write/tv-simple-plus.tv index 77baa7d89..fa8c4cc01 100644 --- a/tests/data/doc_write/tv-simple-plus.tv +++ b/tests/data/doc_write/tv-simple-plus.tv @@ -5,6 +5,7 @@ DataLicense: CC0-1.0 # Package PackageName: some/path PackageDownloadLocation: NOASSERTION +PackageChecksum: SHA1: SOME-SHA1 PackageVerificationCode: SOME code PackageLicenseDeclared: NOASSERTION PackageLicenseConcluded: NOASSERTION diff --git a/tests/data/doc_write/tv-simple.tv b/tests/data/doc_write/tv-simple.tv index a41928039..c88278e7f 100644 --- a/tests/data/doc_write/tv-simple.tv +++ b/tests/data/doc_write/tv-simple.tv @@ -5,6 +5,7 @@ DataLicense: CC0-1.0 # Package PackageName: some/path PackageDownloadLocation: NOASSERTION +PackageChecksum: SHA1: SOME-SHA1 PackageVerificationCode: SOME code PackageLicenseDeclared: NOASSERTION PackageLicenseConcluded: NOASSERTION diff --git a/tests/test_conversion.py b/tests/test_conversion.py index cb9a504b2..ee14c8007 100644 --- a/tests/test_conversion.py +++ b/tests/test_conversion.py @@ -69,42 +69,42 @@ def write_rdf_file(self, document, file_name): def test_tagvalue_rdf(self): doc, error = self.parse_tagvalue_file('data/SPDXTagExample.tag') assert not error - assert doc.validate([]) + assert doc.validate([]) == [] filename = get_temp_file('.rdf') self.write_rdf_file(doc, filename) doc, error = self.parse_rdf_file(filename) assert not error - assert doc.validate([]) + assert doc.validate([]) == [] def test_rdf_rdf(self): doc, error = self.parse_rdf_file('data/SPDXRdfExample.rdf') assert not error - assert doc.validate([]) + assert doc.validate([]) == [] filename = get_temp_file('.rdf') self.write_rdf_file(doc, filename) doc, error = self.parse_rdf_file(filename) assert not error - assert doc.validate([]) + assert doc.validate([]) == [] def test_tagvalue_tagvalue(self): doc, error = self.parse_tagvalue_file('data/SPDXTagExample.tag') assert not error - assert doc.validate([]) + assert doc.validate([]) == [] filename = get_temp_file('.tag') self.write_tagvalue_file(doc, filename) doc, error = self.parse_tagvalue_file(filename) assert not error - assert doc.validate([]) + assert doc.validate([]) == [] def test_rdf_tagvalue(self): doc, error = self.parse_rdf_file('data/SPDXRdfExample.rdf') assert not error - assert doc.validate([]) + assert doc.validate([]) == [] filename = get_temp_file('.tag') self.write_tagvalue_file(doc, filename) doc, error = self.parse_tagvalue_file(filename) assert not error - assert doc.validate([]) + assert doc.validate([]) == [] if __name__ == '__main__': diff --git a/tests/test_document.py b/tests/test_document.py index bb377d790..c7177c366 100644 --- a/tests/test_document.py +++ b/tests/test_document.py @@ -75,10 +75,18 @@ def test_document_validate_failures_returns_informative_messages(self): file1.add_lics(lic1) pack.add_lics_from_file(lic1) messages = [] - is_valid = doc.validate(messages) - assert not is_valid + messages = doc.validate(messages) expected = [ - 'No creators defined, must have at least one.' + 'No creators defined, must have at least one.', + 'Creation info missing created date.', + 'Package checksum must be instance of spdx.checksum.Algorithm', + 'Package verif_code can not be None.', + 'Package cr_text can not be None.', + 'Package must have at least one file.', + 'Package concluded license must be instance of spdx.utils.SPDXNone ' + 'or spdx.utils.NoAssert or spdx.document.License', + 'Package declared license must be instance of spdx.utils.SPDXNone ' + 'or spdx.utils.NoAssert or spdx.document.License' ] assert expected == messages @@ -120,6 +128,7 @@ def _get_lgpl_doc(self, or_later=False): package = doc.package = Package(name='some/path', download_location=NoAssert()) package.cr_text = 'Some copyrught' package.verif_code = 'SOME code' + package.check_sum = Algorithm('SHA1', 'SOME-SHA1') package.license_declared = NoAssert() package.conc_lics = NoAssert()