-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🌱 add csv import test #390
Changes from 19 commits
0ce61f5
b4acc5b
343a266
a3a9f50
723be07
71bd769
4049dd4
55ae1d1
e3a116f
606abcd
69087da
69c4cec
0591e58
477c417
b1fdbc5
2f3835b
619f697
af3d42c
adffc15
9762064
ee0db77
128d807
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,166 @@ | ||
package importcsv | ||
|
||
import ( | ||
"io/ioutil" | ||
"os" | ||
"strconv" | ||
"testing" | ||
"time" | ||
|
||
"github.com/konveyor/tackle2-hub/api" | ||
"github.com/konveyor/tackle2-hub/binding" | ||
"github.com/konveyor/tackle2-hub/test/assert" | ||
) | ||
|
||
func TestImportCSV(t *testing.T) { | ||
for _, r := range TestCases { | ||
t.Run(r.FileName, func(t *testing.T) { | ||
|
||
// Upload CSV. | ||
inputData := api.ImportSummary{} | ||
assert.Must(t, Client.FilePost(api.UploadRoot, r.FileName, &inputData)) | ||
|
||
// Since uploading the CSV happens asynchronously we need to wait for the upload to check Applications and Dependencies. | ||
time.Sleep(time.Second * 3) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we discussed a loop waiting for importsummary until it get process all records (e.g. sleep 1 sec, get importsummary and if valid+invalid count is equal to all application&dependencies count, break from the loop and continue). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is not a blocker, just for info. |
||
|
||
// Check list of Applications. | ||
gotApps, _ := Application.List() | ||
expectedApps := r.ExpectedApplications | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if len(gotApps) != len(expectedApps) { | ||
t.Errorf("Mismatch in number of imported Applications: Expected %d, Actual %d", len(expectedApps), len(gotApps)) | ||
} else { | ||
for i, importedApp := range gotApps { | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if expectedApps[i].Name != importedApp.Name { | ||
t.Errorf("Mismatch in name of imported Application: Expected %s, Actual %s", expectedApps[i].Name, importedApp.Name) | ||
} | ||
if expectedApps[i].Description != importedApp.Description { | ||
t.Errorf("Mismatch in description of imported Application: Expected %s, Actual %s", expectedApps[i].Description, importedApp.Description) | ||
} | ||
if expectedApps[i].Repository.Kind != importedApp.Repository.Kind { | ||
t.Errorf("Mismatch in repository's kind ofimported Application: Expected %s, Actual %s", expectedApps[i].Repository.Kind, importedApp.Repository.Kind) | ||
} | ||
if expectedApps[i].Repository.URL != importedApp.Repository.URL { | ||
t.Errorf("Mismatch in repository's url of imported Application: Expected %s, Actual %s", expectedApps[i].Repository.URL, importedApp.Repository.URL) | ||
} | ||
if expectedApps[i].Binary != importedApp.Binary { | ||
t.Errorf("Mismatch in binary of imported Application: Expected %s, Actual %s", expectedApps[i].Binary, importedApp.Binary) | ||
} | ||
for j, tag := range expectedApps[i].Tags { | ||
if tag.Name != importedApp.Tags[j].Name { | ||
t.Errorf("Mismatch in tag name of imported Application: Expected %s, Actual %s", tag.Name, importedApp.Tags[j].Name) | ||
} | ||
} | ||
if expectedApps[i].BusinessService.Name != importedApp.BusinessService.Name { | ||
t.Errorf("Mismatch in name of the BusinessService of imported Application: Expected %s, Actual %s", expectedApps[i].BusinessService.Name, importedApp.BusinessService.Name) | ||
} | ||
} | ||
} | ||
|
||
// Check list of Dependencies. | ||
gotDeps, _ := Dependency.List() | ||
expectedDeps := r.ExpectedDependencies | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these expectedDeps&expectedApps needed, wouldn't be better use r.Expected... variables? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do that. |
||
if len(gotDeps) != len(expectedDeps) { | ||
t.Errorf("Mismatch in number of imported Dependencies: Expected %d, Actual %d", len(expectedDeps), len(gotDeps)) | ||
} else { | ||
for i, importedDep := range gotDeps { | ||
expectedDep := expectedDeps[i].To.Name | ||
if importedDep.To.Name != expectedDep { | ||
t.Errorf("Mismatch in imported Dependency: Expected %s, Actual %s", expectedDep, importedDep.To.Name) | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
} | ||
} | ||
|
||
// fetch id of CSV file and convert it into required formats | ||
var inputID = strconv.FormatUint(uint64(inputData.ID), 10) // to be used for API compatibility | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
var outputImportSummaries []api.ImportSummary | ||
outputMatchingSummary := api.ImportSummary{} | ||
assert.Should(t, Client.Get(api.SummariesRoot, &outputImportSummaries)) | ||
for _, imp := range outputImportSummaries { | ||
if uint(imp.ID) == inputData.ID { | ||
outputMatchingSummary = imp | ||
} | ||
} | ||
assert.FlatEqual(len(expectedApps)+len(expectedDeps), outputMatchingSummary.ValidCount) | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
// inject import summary id into Summary root | ||
pathForImportSummary := binding.Path(api.SummaryRoot).Inject(binding.Params{api.ID: inputID}) | ||
|
||
// Get summaries of the Input ID. | ||
outputImportSummary := api.ImportSummary{} | ||
assert.Should(t, Client.Get(pathForImportSummary, &outputImportSummary)) | ||
|
||
// Get all imports. | ||
aufi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
var outputImports []api.Import | ||
assert.Should(t, Client.Get(api.ImportsRoot, &outputImports)) | ||
j, k := 0, 0 | ||
for _, imp := range outputImports { | ||
if imp["recordType1"] == 1 && j < len(expectedApps) { | ||
// An Application with no dependencies. | ||
if expectedApps[j].Name != imp["applicationName"] { | ||
t.Errorf("Mismatch in name of import: Expected %s, Actual %s", expectedApps[j].Name, imp["applicationName"]) | ||
} | ||
if expectedApps[j].Description != imp["description"] { | ||
t.Errorf("Mismatch in name of import: Expected %s, Actual %s", expectedApps[j].Description, imp["description"]) | ||
} | ||
if expectedApps[j].BusinessService.Name != imp["businessService"] { | ||
t.Errorf("Mismatch in name of import: Expected %s, Actual %s", expectedApps[j].BusinessService.Name, imp["businessService"]) | ||
} | ||
j++ | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IIUC, shouldn't there be a second part of the condition - if outputImports has more records than expectedApps, the test should fail too. Same for dependencies few lines below. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. outputimports consist of both applications and dependencies thus shouldn't be equal to length of sums? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the CSV for this test would get one more line with an application, but the test (expected applications) was not changed (=> more imports type 1, but not changed len(expectedApps)), it looks to me to pass this part of the test anywa - that was the thing I didn't like. On the other hand there is a check of gotApps len() on line 44 and in this PR we don't have to do deeper rejection tests, so it is likely OK. |
||
if imp["recordType1"] == 2 && k < len(expectedDeps) { | ||
// An Application with Dependencies. | ||
if expectedDeps[k].From.Name != imp["applicationName"] { | ||
t.Errorf("Mismatch in name of import: Expected %s, Actual %s", expectedDeps[k].From.Name, imp["applicationName"]) | ||
} | ||
if expectedDeps[k].To.Name != imp["dependency"] { | ||
t.Errorf("Mismatch in name of import: Expected %s, Actual %s", expectedDeps[k].To.Name, imp["dependency"]) | ||
} | ||
k++ | ||
} | ||
} | ||
|
||
// Download the csv. | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
pathToGotCSV := "downloadcsv.csv" | ||
err := Client.FileGet(api.DownloadRoot, pathToGotCSV) | ||
if err != nil { | ||
t.Errorf("Error downloading CSV: %v", err) | ||
} | ||
|
||
// Read the got CSV file. | ||
gotCSV, err := ioutil.ReadFile(pathToGotCSV) | ||
if err != nil { | ||
t.Errorf("Error reading CSV: %s", pathToGotCSV) | ||
} | ||
gotCSVString := string(gotCSV) | ||
|
||
// Read the expected CSV file. | ||
expectedCSV, err := ioutil.ReadFile(r.FileName) | ||
if err != nil { | ||
t.Errorf("Error reading CSV: %s", r.FileName) | ||
} | ||
expectedCSVString := string(expectedCSV) | ||
|
||
if gotCSVString != expectedCSVString { | ||
t.Errorf("The CSV files have different content %s and %s", gotCSVString, expectedCSVString) | ||
} | ||
|
||
// Remove the CSV file created. | ||
err = os.Remove(pathToGotCSV) | ||
if err != nil { | ||
t.Errorf(err.Error()) | ||
} | ||
|
||
// Delete import summary | ||
assert.Should(t, Client.Delete(pathForImportSummary)) | ||
|
||
// Delete all imports | ||
id := 1 | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for id <= len(expectedApps)+len(expectedDeps) { | ||
pathForImport := binding.Path(api.ImportRoot).Inject(binding.Params{api.ID: id}) | ||
assert.Should(t, Client.Delete(pathForImport)) | ||
id++ | ||
} | ||
khareyash05 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package importcsv | ||
|
||
import ( | ||
"github.com/konveyor/tackle2-hub/binding" | ||
"github.com/konveyor/tackle2-hub/test/api/client" | ||
) | ||
|
||
var ( | ||
RichClient *binding.RichClient | ||
Client *binding.Client | ||
Application binding.Application | ||
Dependency binding.Dependency | ||
) | ||
|
||
func init() { | ||
// Prepare RichClient and login to Hub API (configured from env variables). | ||
RichClient = client.PrepareRichClient() | ||
|
||
// Access REST client directly | ||
Client = RichClient.Client | ||
|
||
// Access Application directly | ||
Application = RichClient.Application | ||
|
||
// Access Dependency directly | ||
Dependency = RichClient.Dependency | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,137 @@ | ||
package importcsv | ||
|
||
import ( | ||
"github.com/konveyor/tackle2-hub/api" | ||
) | ||
|
||
type TestCase struct { | ||
FileName string | ||
ExpectedApplications []api.Application | ||
ExpectedDependencies []api.Dependency | ||
} | ||
|
||
var ( | ||
TestCases = []TestCase{ | ||
{ | ||
FileName: "template_application_import.csv", | ||
ExpectedApplications: []api.Application{ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This array looks good, please go ahead and add Application fields data from the CSV. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the data in this CSV is supposed to be equivalent to what is being described in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Ya even I think so because we are checking that if all the entities are being uploaded or not thus have to check them all |
||
{ | ||
Name: "Customers", | ||
Description: "Legacy Customers management service", | ||
Bucket: &api.Ref{}, | ||
Repository: &api.Repository{ | ||
Kind: "git", | ||
URL: "https://git-acme.local/customers.git", | ||
Branch: "", | ||
Tag: "", | ||
Path: "", | ||
}, | ||
Binary: "corp.acme.demo:customers-tomcat:0.0.1-SNAPSHOT:war", | ||
Tags: []api.TagRef{ | ||
{ | ||
Name: "Oracle", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "Java", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "RHEL 8", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "Tomcat", | ||
Source: "", | ||
}, | ||
}, | ||
BusinessService: &api.Ref{ | ||
Name: "Retail", | ||
}, | ||
}, | ||
{ | ||
Name: "Inventory", | ||
Description: "Inventory service", | ||
Bucket: &api.Ref{}, | ||
Repository: &api.Repository{ | ||
Kind: "git", | ||
URL: "https://git-acme.local/inventory.git", | ||
Branch: "", | ||
Tag: "", | ||
Path: "", | ||
}, | ||
Binary: "corp.acme.demo:inventory:0.1.1-SNAPSHOT:war", | ||
Tags: []api.TagRef{ | ||
{ | ||
Name: "PostgreSQL", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "Java", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "RHEL 8", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "Quarkus", | ||
Source: "", | ||
}, | ||
}, | ||
BusinessService: &api.Ref{ | ||
Name: "Retail", | ||
}, | ||
}, | ||
{ | ||
Name: "Gateway", | ||
Description: "API Gateway", | ||
Bucket: &api.Ref{}, | ||
Repository: &api.Repository{ | ||
Kind: "git", | ||
URL: "https://git-acme.local/gateway.git", | ||
Branch: "", | ||
Tag: "", | ||
Path: "", | ||
}, | ||
Binary: "corp.acme.demo:gateway:0.1.1-SNAPSHOT:war", | ||
Tags: []api.TagRef{ | ||
{ | ||
Name: "Java", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "RHEL 8", | ||
Source: "", | ||
}, | ||
{ | ||
Name: "Spring Boot", | ||
Source: "", | ||
}, | ||
}, | ||
BusinessService: &api.Ref{ | ||
Name: "Retail", | ||
}, | ||
}, | ||
}, | ||
ExpectedDependencies: []api.Dependency{ | ||
{ | ||
To: api.Ref{ | ||
Name: "Inventory", | ||
}, | ||
From: api.Ref{ | ||
Name: "Gateway", | ||
}, | ||
}, | ||
{ | ||
To: api.Ref{ | ||
Name: "Customers", | ||
}, | ||
From: api.Ref{ | ||
Name: "Gateway", | ||
}, | ||
}, | ||
}, | ||
}, | ||
} | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
Record Type 1,Application Name,Description,Comments,Business Service,Dependency,Dependency Direction,Binary Group,Binary Artifact,Binary Version,Binary Packaging,Repository Type,Repository URL,Repository Branch,Repository Path,Tag Category 1,Tag 1,Tag Category 2,Tag 2,Tag Category 3,Tag 3,Tag Category 4,Tag 4,Tag Category 5,Tag 5,Tag Category 6,Tag 6,Tag Category 7,Tag 7,Tag Category 8,Tag 8,Tag Category 9,Tag 9,Tag Category 10,Tag 10,Tag Category 11,Tag 11,Tag Category 12,Tag 12,Tag Category 13,Tag 13,Tag Category 14,Tag 14,Tag Category 15,Tag 15,Tag Category 16,Tag 16,Tag Category 17,Tag 17,Tag Category 18,Tag 18,Tag Category 19,Tag 19,Tag Category 20,Tag 20 | ||
1,Customers,Legacy Customers management service,,Retail,,,corp.acme.demo,customers-tomcat,0.0.1-SNAPSHOT,war,git,https://git-acme.local/customers.git,,,Operating System,RHEL 8,Database,Oracle,Language,Java,Runtime,Tomcat,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
1,Inventory,Inventory service,,Retail,,,corp.acme.demo,inventory,0.1.1-SNAPSHOT,war,git,https://git-acme.local/inventory.git,,,Operating System,RHEL 8,Database,Postgresql,Language,Java,Runtime,Quarkus,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
1,Gateway,API Gateway,,Retail,,,corp.acme.demo,gateway,0.1.1-SNAPSHOT,war,git,https://git-acme.local/gateway.git,,,Operating System,RHEL 8,,,Language,Java,Runtime,Spring Boot,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,, | ||
2,Gateway,,,,Inventory,southbound | ||
2,Gateway,,,,Customers,southbound |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit, but not needed change, please remove trailing spaces.