Skip to content
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

Merged
merged 22 commits into from
Jul 3, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions binding/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ import (
"encoding/json"
"errors"
"fmt"
"github.com/gin-gonic/gin/binding"
liberr "github.com/jortel/go-utils/error"
"github.com/konveyor/tackle2-hub/api"
"io"
"mime/multipart"
"net"
Expand All @@ -22,6 +19,10 @@ import (
"path/filepath"
"strings"
"time"

"github.com/gin-gonic/gin/binding"
liberr "github.com/jortel/go-utils/error"
"github.com/konveyor/tackle2-hub/api"
)

const (
Expand Down Expand Up @@ -368,7 +369,7 @@ func (r *Client) BucketPut(source, destination string) (err error) {
if isDir {
err = r.putDir(part, source)
} else {
err = r.putFile(part, source)
err = r.loadFile(part, source)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please correct me if I'm wrong, but the changes related to putFile -> loadFile methods doesn't seem to be needed since you created FilePost method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had actually changed the name of the function from putFile to loadFile to make it generic. Thus updated the same everywhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood, but this change came in time when there was another function calling the loadFile, which is gone since you added FilePost method (which is a better solution for this). So the putFile is called only from BucketPut, so I'd would not recommend changing it (as actually unrelated code to this test/PR).

}
}()
return
Expand Down Expand Up @@ -447,6 +448,29 @@ func (r *Client) FilePut(path, source string, object interface{}) (err error) {
return
}

//
// FilePost uploads a file.
// Returns the created File resource.
func (r *Client) FilePost(path, source string, object interface{}) (err error) {
isDir, nErr := r.IsDir(source, true)
if nErr != nil {
err = nErr
return
}
if isDir {
err = liberr.New("Must be regular file.")
return
}
fields := []Field{
{
Name: api.FileField,
Path: source,
},
}
err = r.FileSend(path, http.MethodPost, fields, object)
return
}

//
// FileSend sends file upload from.
func (r *Client) FileSend(path, method string, fields []Field, object interface{}) (err error) {
Expand Down Expand Up @@ -646,8 +670,8 @@ func (r *Client) getFile(body io.Reader, path, output string) (err error) {
}

//
// putFile uploads plain file.
func (r *Client) putFile(writer io.Writer, input string) (err error) {
// loadFile uploads a file.
func (r *Client) loadFile(writer io.Writer, input string) (err error) {
file, err := os.Open(input)
if err != nil {
err = liberr.Wrap(err)
Expand Down
78 changes: 78 additions & 0 deletions test/api/importcsv/api_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
package importcsv

import (
"strconv"
"testing"

"github.com/konveyor/tackle2-hub/api"
"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.SummariesRoot+"/upload", r.FileName, &inputData))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the right way creating the path, please follow what you had in your previous PR https://github.com/konveyor/tackle2-hub/pull/375/files#diff-6ded4e5e60480daa4db7cf0740809d6b470d937e85d1b674b02777560fed1a38R25


// 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
assert.FlatEqual(expectedApps[i].Name, importedApp.Name)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These assertions return true/false, but actually do nothing with the test result. Please look to the FlatEqual method body and to your previous PR to its usage (e.g. https://github.com/konveyor/tackle2-hub/blob/main/test/api/dependency/api_test.go#L34-L36)

In some cases, it might be better use == instead of FlatEqual function (expectedApps[i].Name, importedApp.Name => if expectedApps[i].Name != importedApp.Name {error...})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do that!

assert.FlatEqual(expectedApps[i].Description, importedApp.Description)
assert.FlatEqual(expectedApps[i].Repository.Kind, importedApp.Repository.Kind)
assert.FlatEqual(expectedApps[i].Repository.URL, importedApp.Repository.URL)
assert.FlatEqual(expectedApps[i].Binary, importedApp.Binary)
for j, tag := range expectedApps[i].Tags {
assert.FlatEqual(tag.Name, importedApp.Tags[j].Name)
}
assert.FlatEqual(expectedApps[i].BusinessService.Name, importedApp.BusinessService.Name)
}
}

// Check list of Dependencies.
gotDeps, _ := Dependency.List()
expectedDeps := r.ExpectedDependencies
Copy link
Member

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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

// Get summaries of the Input ID.
outputImportSummary := api.ImportSummary{}
assert.Should(t, Client.Get(api.SummariesRoot+"/"+inputID, &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))

// Get import of the specific Input Id.
outputImport := api.Import{}
assert.Should(t, Client.Get(api.ImportsRoot+"/"+inputID, &outputImport))
})
}
}
27 changes: 27 additions & 0 deletions test/api/importcsv/pkg.go
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
}
137 changes: 137 additions & 0 deletions test/api/importcsv/samples.go
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{
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Member

Choose a reason for hiding this comment

The 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 template_application_import.csv?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 template_application_import.csv?

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",
},
},
},
},
}
)
6 changes: 6 additions & 0 deletions test/api/importcsv/template_application_import.csv
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