From 4efac7b1447c34fd10fa3b5a8cdb1e690d2b6be6 Mon Sep 17 00:00:00 2001 From: specter25 Date: Wed, 12 May 2021 19:26:25 +0530 Subject: Structural modifications in error checks -- Added file name display in error messages - Structured the tests in respective files Signed-off-by: specter25 --- tvloader/parser2v1/parse_creation_info.go | 2 +- tvloader/parser2v1/parse_file.go | 4 +-- tvloader/parser2v1/parse_file_test.go | 37 ++++++++++----------------- tvloader/parser2v1/parse_package.go | 2 +- tvloader/parser2v1/parse_package_test.go | 12 --------- tvloader/parser2v1/parse_snippet.go | 8 +++--- tvloader/parser2v1/parser.go | 4 +-- tvloader/parser2v1/parser_test.go | 30 ++++++++++++++++++++++ tvloader/parser2v2/parse_creation_info.go | 2 +- tvloader/parser2v2/parse_file.go | 4 +-- tvloader/parser2v2/parse_file_test.go | 42 +++++++++++++------------------ tvloader/parser2v2/parse_package.go | 2 +- tvloader/parser2v2/parse_package_test.go | 12 --------- tvloader/parser2v2/parse_snippet.go | 8 +++--- tvloader/parser2v2/parser.go | 4 +-- tvloader/parser2v2/parser_test.go | 30 ++++++++++++++++++++++ 16 files changed, 110 insertions(+), 93 deletions(-) (limited to 'tvloader') diff --git a/tvloader/parser2v1/parse_creation_info.go b/tvloader/parser2v1/parse_creation_info.go index c492884..dd9a004 100644 --- a/tvloader/parser2v1/parse_creation_info.go +++ b/tvloader/parser2v1/parse_creation_info.go @@ -78,7 +78,7 @@ func (parser *tvParser2_1) parsePairFromCreationInfo2_1(tag string, value string case "PackageName": // Error if last file does not has FileSPDXId if parser.file != nil && parser.file.FileSPDXIdentifier == spdx.ElementID("") { - return fmt.Errorf("invalid file without a package SPDX identifier") + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } parser.st = psPackage2_1 parser.pkg = &spdx.Package2_1{ diff --git a/tvloader/parser2v1/parse_file.go b/tvloader/parser2v1/parse_file.go index 772e36e..7347384 100644 --- a/tvloader/parser2v1/parse_file.go +++ b/tvloader/parser2v1/parse_file.go @@ -20,7 +20,7 @@ func (parser *tvParser2_1) parsePairFromFile2_1(tag string, value string) error case "FileName": // check if the previous file contained a spdxId or not if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_1 { - return fmt.Errorf("invalid file without a file SPDX identifier") + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } parser.file = &spdx.File2_1{} parser.file.FileName = value @@ -28,7 +28,7 @@ func (parser *tvParser2_1) parsePairFromFile2_1(tag string, value string) error case "PackageName": // check if the previous file contained a spdxId or not if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_1 { - return fmt.Errorf("invalid file without a file SPDX identifier") + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } parser.st = psPackage2_1 parser.file = nil diff --git a/tvloader/parser2v1/parse_file_test.go b/tvloader/parser2v1/parse_file_test.go index 481d577..e302a53 100644 --- a/tvloader/parser2v1/parse_file_test.go +++ b/tvloader/parser2v1/parse_file_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/spdx/tools-golang/spdx" - "github.com/spdx/tools-golang/tvloader/reader" ) // ===== Parser file section state change tests ===== @@ -887,44 +886,34 @@ func TestParser2_1FailsIfArtifactURIBeforeArtifactName(t *testing.T) { } func TestParser2_1FilesWithoutSpdxIdThrowError(t *testing.T) { - // case 1 - // Last unpackaged file no packages in doc - // Last file of last package in the doc - tvPairs := []reader.TagValuePair{ - {Tag: "SPDXVersion", Value: "SPDX-2.1"}, - {Tag: "DataLicense", Value: "CC0-1.0"}, - {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, - {Tag: "FileName", Value: "f1"}, - } - _, err := ParseTagValues(tvPairs) - if err == nil { - t.Errorf("files withoutSpdx Identifiers getting accepted") + // case 1 : The previous file (packaged or unpackaged does not contain spdxID) + parser1 := tvParser2_1{ + doc: &spdx.Document2_1{Packages: map[spdx.ElementID]*spdx.Package2_1{}}, + st: psFile2_1, + file: &spdx.File2_1{FileName: "FileName"}, } - // case 2 : The previous file (packaged or unpackaged does not contain spdxID) - tvPairs = append(tvPairs, reader.TagValuePair{Tag: "FileName", Value: "f2"}) - _, err = ParseTagValues(tvPairs) + err := parser1.parsePair2_1("FileName", "f2") if err == nil { - t.Errorf("%s", err) + t.Errorf("files withoutSpdx Identifiers getting accepted") } - // case 3 : Invalid file with snippet - // Last unpackaged file before the packges start + // case 2 : Invalid file with snippet + // Last unpackaged file before the snippet start // Last file of a package and New package starts sid1 := spdx.ElementID("s1") - parser := tvParser2_1{ + parser2 := tvParser2_1{ doc: &spdx.Document2_1{}, st: psCreationInfo2_1, } fileName := "f2.txt" - _ = parser.parsePair2_1("FileName", fileName) - _ = parser.parsePair2_1("SnippetSPDXID", string(sid1)) - err = parser.parsePair2_1("PackageName", "p2") + _ = parser2.parsePair2_1("FileName", fileName) + err = parser2.parsePair2_1("SnippetSPDXID", string(sid1)) if err == nil { t.Errorf("files withoutSpdx Identifiers getting accepted") } - // case 4 : Invalid File without snippets + // case 3 : Invalid File without snippets // Last unpackaged file before the packges start // Last file of a package and New package starts parser3 := tvParser2_1{ diff --git a/tvloader/parser2v1/parse_package.go b/tvloader/parser2v1/parse_package.go index 2ac355a..eb96f4c 100644 --- a/tvloader/parser2v1/parse_package.go +++ b/tvloader/parser2v1/parse_package.go @@ -22,7 +22,7 @@ func (parser *tvParser2_1) parsePairFromPackage2_1(tag string, value string) err if parser.pkg == nil || parser.pkg.PackageName != "" { // check if the previous package contained a spdxId or not if parser.pkg != nil && parser.pkg.PackageSPDXIdentifier == nullSpdxElementId2_1 { - return fmt.Errorf("invalid package without a package SPDX identifier") + return fmt.Errorf("package with PackageName %s does not have SPDX identifier", parser.pkg.PackageName) } parser.pkg = &spdx.Package2_1{ FilesAnalyzed: true, diff --git a/tvloader/parser2v1/parse_package_test.go b/tvloader/parser2v1/parse_package_test.go index 3aa0ddb..e43727b 100644 --- a/tvloader/parser2v1/parse_package_test.go +++ b/tvloader/parser2v1/parse_package_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/spdx/tools-golang/spdx" - "github.com/spdx/tools-golang/tvloader/reader" ) // ===== Parser package section state change tests ===== @@ -1088,20 +1087,9 @@ func TestParser2_1PackageWithoutSpdxIdentifierThrowsError(t *testing.T) { t.Errorf("expected 1 package, got %d", len(parser.doc.Packages)) } - // Case 2: Checks the Last package pkgName := "p2" err := parser.parsePair2_1("PackageName", pkgName) if err == nil { t.Errorf("packages withoutSpdx Identifiers getting accepted") } - tvPairs := []reader.TagValuePair{ - {Tag: "SPDXVersion", Value: "SPDX-2.1"}, - {Tag: "DataLicense", Value: "CC0-1.0"}, - {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, - {Tag: "PackageName", Value: "p1"}, - } - _, err = ParseTagValues(tvPairs) - if err == nil { - t.Errorf("packages withoutSpdx Identifiers getting accepted") - } } diff --git a/tvloader/parser2v1/parse_snippet.go b/tvloader/parser2v1/parse_snippet.go index f44c964..b8aafad 100644 --- a/tvloader/parser2v1/parse_snippet.go +++ b/tvloader/parser2v1/parse_snippet.go @@ -13,6 +13,10 @@ func (parser *tvParser2_1) parsePairFromSnippet2_1(tag string, value string) err switch tag { // tag for creating new snippet section case "SnippetSPDXID": + // check here whether the file containe + if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_1 { + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) + } parser.snippet = &spdx.Snippet2_1{} eID, err := extractElementID(value) if err != nil { @@ -34,10 +38,6 @@ func (parser *tvParser2_1) parsePairFromSnippet2_1(tag string, value string) err // tag for creating new package section and going back to parsing Package case "PackageName": parser.st = psPackage2_1 - // check here whether the last file of the previous package contained the FileSpdxIdentifier - if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_1 { - return fmt.Errorf("invalid file without a file SPDX identifier") - } parser.file = nil parser.snippet = nil return parser.parsePairFromPackage2_1(tag, value) diff --git a/tvloader/parser2v1/parser.go b/tvloader/parser2v1/parser.go index 6504474..f4a5ae1 100644 --- a/tvloader/parser2v1/parser.go +++ b/tvloader/parser2v1/parser.go @@ -21,10 +21,10 @@ func ParseTagValues(tvs []reader.TagValuePair) (*spdx.Document2_1, error) { } } if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_1 { - return nil, fmt.Errorf("invalid file without a file SPDX identifier") + return nil, fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } if parser.pkg != nil && parser.pkg.PackageSPDXIdentifier == nullSpdxElementId2_1 { - return nil, fmt.Errorf("invalid package without a package SPDX identifier") + return nil, fmt.Errorf("package with PackageName %s does not have SPDX identifier", parser.pkg.PackageName) } return parser.doc, nil diff --git a/tvloader/parser2v1/parser_test.go b/tvloader/parser2v1/parser_test.go index 4a61828..0678fd7 100644 --- a/tvloader/parser2v1/parser_test.go +++ b/tvloader/parser2v1/parser_test.go @@ -77,3 +77,33 @@ func TestParser2_1StartFailsToParseIfInInvalidState(t *testing.T) { t.Errorf("expected non-nil error, got nil") } } + +func TestParser2_1FilesWithoutSpdxIdThrowErrorAtCompleteParse(t *testing.T) { + // case + // Last unpackaged file no packages in doc + // Last file of last package in the doc + tvPairs := []reader.TagValuePair{ + {Tag: "SPDXVersion", Value: "SPDX-2.1"}, + {Tag: "DataLicense", Value: "CC0-1.0"}, + {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, + {Tag: "FileName", Value: "f1"}, + } + _, err := ParseTagValues(tvPairs) + if err == nil { + t.Errorf("files withoutSpdx Identifiers getting accepted") + } +} + +func TestParser2_1PackageWithoutSpdxIdThrowErrorAtCompleteParse(t *testing.T) { + // case : Checks the last package + tvPairs := []reader.TagValuePair{ + {Tag: "SPDXVersion", Value: "SPDX-2.1"}, + {Tag: "DataLicense", Value: "CC0-1.0"}, + {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, + {Tag: "PackageName", Value: "p1"}, + } + _, err := ParseTagValues(tvPairs) + if err == nil { + t.Errorf("packages withoutSpdx Identifiers getting accepted") + } +} diff --git a/tvloader/parser2v2/parse_creation_info.go b/tvloader/parser2v2/parse_creation_info.go index 930ff00..8e7d270 100644 --- a/tvloader/parser2v2/parse_creation_info.go +++ b/tvloader/parser2v2/parse_creation_info.go @@ -78,7 +78,7 @@ func (parser *tvParser2_2) parsePairFromCreationInfo2_2(tag string, value string case "PackageName": // Error if last file does not has FileSPDXId if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_2 { - return fmt.Errorf("invalid file without a package SPDX identifier") + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } parser.st = psPackage2_2 parser.pkg = &spdx.Package2_2{ diff --git a/tvloader/parser2v2/parse_file.go b/tvloader/parser2v2/parse_file.go index 7e004e9..ad2d32c 100644 --- a/tvloader/parser2v2/parse_file.go +++ b/tvloader/parser2v2/parse_file.go @@ -20,7 +20,7 @@ func (parser *tvParser2_2) parsePairFromFile2_2(tag string, value string) error case "FileName": // check if the previous file contained a spdxId or not if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_2 { - return fmt.Errorf("invalid file without a file SPDX identifier") + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } parser.file = &spdx.File2_2{} parser.file.FileName = value @@ -29,7 +29,7 @@ func (parser *tvParser2_2) parsePairFromFile2_2(tag string, value string) error parser.st = psPackage2_2 // check if the previous file containes a spdxId or not if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_2 { - return fmt.Errorf("invalid file without a file SPDX identifier") + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } parser.file = nil return parser.parsePairFromPackage2_2(tag, value) diff --git a/tvloader/parser2v2/parse_file_test.go b/tvloader/parser2v2/parse_file_test.go index f5234a2..6b8f878 100644 --- a/tvloader/parser2v2/parse_file_test.go +++ b/tvloader/parser2v2/parse_file_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/spdx/tools-golang/spdx" - "github.com/spdx/tools-golang/tvloader/reader" ) // ===== Parser file section state change tests ===== @@ -920,44 +919,34 @@ func TestParser2_2FailsIfArtifactURIBeforeArtifactName(t *testing.T) { } func TestParser2_2FilesWithoutSpdxIdThrowError(t *testing.T) { - // case 1 - // Last unpackaged file no packages in doc - // Last file of last package in the doc - tvPairs := []reader.TagValuePair{ - {Tag: "SPDXVersion", Value: "SPDX-2.2"}, - {Tag: "DataLicense", Value: "CC0-1.0"}, - {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, - {Tag: "FileName", Value: "f1"}, - } - _, err := ParseTagValues(tvPairs) - if err == nil { - t.Errorf("files withoutSpdx Identifiers getting accepted") + // case 1 : The previous file (packaged or unpackaged does not contain spdxID) + parser1 := tvParser2_2{ + doc: &spdx.Document2_2{Packages: map[spdx.ElementID]*spdx.Package2_2{}}, + st: psFile2_2, + file: &spdx.File2_2{FileName: "FileName"}, } - // case 2 : The previous file (packaged or unpackaged does not contain spdxID) - tvPairs = append(tvPairs, reader.TagValuePair{Tag: "FileName", Value: "f2"}) - _, err = ParseTagValues(tvPairs) + err := parser1.parsePair2_2("FileName", "f2") if err == nil { - t.Errorf("%s", err) + t.Errorf("files withoutSpdx Identifiers getting accepted") } - // case 3 : Invalid file with snippet - // Last unpackaged file before the packges start + // case 2 : Invalid file with snippet + // Last unpackaged file before the snippet start // Last file of a package and New package starts sid1 := spdx.ElementID("s1") - parser := tvParser2_2{ + parser2 := tvParser2_2{ doc: &spdx.Document2_2{}, st: psCreationInfo2_2, } fileName := "f2.txt" - _ = parser.parsePair2_2("FileName", fileName) - _ = parser.parsePair2_2("SnippetSPDXID", string(sid1)) - err = parser.parsePair2_2("PackageName", "p2") + _ = parser2.parsePair2_2("FileName", fileName) + err = parser2.parsePair2_2("SnippetSPDXID", string(sid1)) if err == nil { t.Errorf("files withoutSpdx Identifiers getting accepted") } - // case 4 : Invalid File without snippets + // case 3 : Invalid File without snippets // Last unpackaged file before the packges start // Last file of a package and New package starts parser3 := tvParser2_2{ @@ -965,7 +954,10 @@ func TestParser2_2FilesWithoutSpdxIdThrowError(t *testing.T) { st: psCreationInfo2_2, } fileName = "f3.txt" - _ = parser3.parsePair2_2("FileName", fileName) + err = parser3.parsePair2_2("FileName", fileName) + if err != nil { + t.Errorf("%s", err) + } err = parser3.parsePair2_2("PackageName", "p2") if err == nil { t.Errorf("files withoutSpdx Identifiers getting accepted") diff --git a/tvloader/parser2v2/parse_package.go b/tvloader/parser2v2/parse_package.go index 3d16272..a9bcb6c 100644 --- a/tvloader/parser2v2/parse_package.go +++ b/tvloader/parser2v2/parse_package.go @@ -22,7 +22,7 @@ func (parser *tvParser2_2) parsePairFromPackage2_2(tag string, value string) err if parser.pkg == nil || parser.pkg.PackageName != "" { // check if the previous package contained a spdxId or not if parser.pkg != nil && parser.pkg.PackageSPDXIdentifier == nullSpdxElementId2_2 { - return fmt.Errorf("invalid package without a package SPDX identifier") + return fmt.Errorf("package with PackageName %s does not have SPDX identifier", parser.pkg.PackageName) } parser.pkg = &spdx.Package2_2{ FilesAnalyzed: true, diff --git a/tvloader/parser2v2/parse_package_test.go b/tvloader/parser2v2/parse_package_test.go index 6ed5dce..1028a64 100644 --- a/tvloader/parser2v2/parse_package_test.go +++ b/tvloader/parser2v2/parse_package_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/spdx/tools-golang/spdx" - "github.com/spdx/tools-golang/tvloader/reader" ) // ===== Parser package section state change tests ===== @@ -1122,20 +1121,9 @@ func TestParser2_2PackageWithoutSpdxIdentifierThrowsError(t *testing.T) { t.Errorf("expected 1 package, got %d", len(parser.doc.Packages)) } - // Case 2: Checks the Last package pkgName := "p2" err := parser.parsePair2_2("PackageName", pkgName) if err == nil { t.Errorf("packages withoutSpdx Identifiers getting accepted") } - tvPairs := []reader.TagValuePair{ - {Tag: "SPDXVersion", Value: "SPDX-2.2"}, - {Tag: "DataLicense", Value: "CC0-1.0"}, - {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, - {Tag: "PackageName", Value: "p1"}, - } - _, err = ParseTagValues(tvPairs) - if err == nil { - t.Errorf("packages withoutSpdx Identifiers getting accepted") - } } diff --git a/tvloader/parser2v2/parse_snippet.go b/tvloader/parser2v2/parse_snippet.go index 59b8472..6ea1f5d 100644 --- a/tvloader/parser2v2/parse_snippet.go +++ b/tvloader/parser2v2/parse_snippet.go @@ -13,6 +13,10 @@ func (parser *tvParser2_2) parsePairFromSnippet2_2(tag string, value string) err switch tag { // tag for creating new snippet section case "SnippetSPDXID": + // check here whether the file containe + if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_2 { + return fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) + } parser.snippet = &spdx.Snippet2_2{} eID, err := extractElementID(value) if err != nil { @@ -34,10 +38,6 @@ func (parser *tvParser2_2) parsePairFromSnippet2_2(tag string, value string) err // tag for creating new package section and going back to parsing Package case "PackageName": parser.st = psPackage2_2 - // check here whether the last file of the previous package contained the FileSpdxIdentifier - if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_2 { - return fmt.Errorf("invalid file without a file SPDX identifier") - } parser.file = nil parser.snippet = nil return parser.parsePairFromPackage2_2(tag, value) diff --git a/tvloader/parser2v2/parser.go b/tvloader/parser2v2/parser.go index 4f47de5..9886874 100644 --- a/tvloader/parser2v2/parser.go +++ b/tvloader/parser2v2/parser.go @@ -21,10 +21,10 @@ func ParseTagValues(tvs []reader.TagValuePair) (*spdx.Document2_2, error) { } } if parser.file != nil && parser.file.FileSPDXIdentifier == nullSpdxElementId2_2 { - return nil, fmt.Errorf("invalid file without a file SPDX identifier") + return nil, fmt.Errorf("file with FileName %s does not have SPDX identifier", parser.file.FileName) } if parser.pkg != nil && parser.pkg.PackageSPDXIdentifier == nullSpdxElementId2_2 { - return nil, fmt.Errorf("invalid package without a package SPDX identifier") + return nil, fmt.Errorf("package with PackageName %s does not have SPDX identifier", parser.pkg.PackageName) } return parser.doc, nil } diff --git a/tvloader/parser2v2/parser_test.go b/tvloader/parser2v2/parser_test.go index 7eec49c..8393358 100644 --- a/tvloader/parser2v2/parser_test.go +++ b/tvloader/parser2v2/parser_test.go @@ -77,3 +77,33 @@ func TestParser2_2StartFailsToParseIfInInvalidState(t *testing.T) { t.Errorf("expected non-nil error, got nil") } } + +func TestParser2_2FilesWithoutSpdxIdThrowErrorAtCompleteParse(t *testing.T) { + // case + // Last unpackaged file no packages in doc + // Last file of last package in the doc + tvPairs := []reader.TagValuePair{ + {Tag: "SPDXVersion", Value: "SPDX-2.2"}, + {Tag: "DataLicense", Value: "CC0-1.0"}, + {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, + {Tag: "FileName", Value: "f1"}, + } + _, err := ParseTagValues(tvPairs) + if err == nil { + t.Errorf("files withoutSpdx Identifiers getting accepted") + } +} + +func TestParser2_2PackageWithoutSpdxIdThrowErrorAtCompleteParse(t *testing.T) { + // case : Checks the last package + tvPairs := []reader.TagValuePair{ + {Tag: "SPDXVersion", Value: "SPDX-2.2"}, + {Tag: "DataLicense", Value: "CC0-1.0"}, + {Tag: "SPDXID", Value: "SPDXRef-DOCUMENT"}, + {Tag: "PackageName", Value: "p1"}, + } + _, err := ParseTagValues(tvPairs) + if err == nil { + t.Errorf("packages withoutSpdx Identifiers getting accepted") + } +} -- cgit v1.2.3