Add Code Review Changes

pull/8423/head
ys.achinta 5 years ago
parent 9503263ac5
commit 34c6d04343

@ -1,18 +1,9 @@
package api
import (
"github.com/stretchr/testify/mock"
"helm.sh/helm/v3/pkg/cli"
)
type chartloader interface {
LocateChart(name string, settings *cli.EnvSettings) (string, error)
}
type MockChartLoader struct{ mock.Mock }
func (m *MockChartLoader) LocateChart(name string, settings *cli.EnvSettings) (string, error) {
args := m.Called(name, settings)
return args.String(0), args.Error(1)
}

@ -16,7 +16,7 @@ type InstallRequest struct {
type InstallResponse struct {
Error string `json:"error,omitempty"`
Status string `json:"status"`
Status string `json:"Status"`
}
// RODO: we could use interface as well if everything's in same package
@ -39,7 +39,7 @@ func Install(svc Service) http.Handler {
w.WriteHeader(http.StatusInternalServerError)
return
}
response.Status = res.status
response.Status = res.Status
if err := json.NewEncoder(w).Encode(&response); err != nil {
logger.Errorf("[Install] error writing response %v", err)
w.WriteHeader(http.StatusInternalServerError)

@ -2,6 +2,7 @@ package api_test
import (
"fmt"
"helm.sh/helm/v3/pkg/chart"
"io/ioutil"
"net/http"
"net/http/httptest"
@ -22,21 +23,39 @@ type InstallerTestSuite struct {
suite.Suite
recorder *httptest.ResponseRecorder
server *httptest.Server
mockInstall *api.MockInstall
mockChartLoader *api.MockChartLoader
mockList *api.MockList
mockInstall *mockInstall
mockChartLoader *mockChartLoader
mockList *mockList
appConfig *cli.EnvSettings
}
type mockInstall struct{ mock.Mock }
func (m *mockInstall) SetConfig(cfg api.InstallConfig) {
m.Called(cfg)
}
func (m *mockInstall) Run(c *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
args := m.Called(c, vals)
return args.Get(0).(*release.Release), args.Error(1)
}
type mockChartLoader struct{ mock.Mock }
func (m *mockChartLoader) LocateChart(name string, settings *cli.EnvSettings) (string, error) {
args := m.Called(name, settings)
return args.String(0), args.Error(1)
}
func (s *InstallerTestSuite) SetupSuite() {
logger.Setup("default")
}
func (s *InstallerTestSuite) SetupTest() {
s.recorder = httptest.NewRecorder()
s.mockInstall = new(api.MockInstall)
s.mockChartLoader = new(api.MockChartLoader)
s.mockList = new(api.MockList)
s.mockInstall = new(mockInstall)
s.mockChartLoader = new(mockChartLoader)
s.mockList = new(mockList)
s.appConfig = &cli.EnvSettings{
RepositoryConfig: "./testdata/helm",
PluginsDirectory: "./testdata/helm/plugin",
@ -64,7 +83,7 @@ func (s *InstallerTestSuite) TestShouldReturnDeployedStatusOnSuccessfulInstall()
resp, err := http.DefaultClient.Do(req)
assert.Equal(s.T(), 200, resp.StatusCode)
expectedResponse := `{"status":"deployed"}` + "\n"
expectedResponse := `{"Status":"deployed"}` + "\n"
respBody, _ := ioutil.ReadAll(resp.Body)
assert.Equal(s.T(), expectedResponse, string(respBody))
require.NoError(s.T(), err)

@ -1,7 +1,6 @@
package api
import (
"github.com/stretchr/testify/mock"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/chart"
"helm.sh/helm/v3/pkg/release"
@ -24,14 +23,3 @@ func (i *install) SetConfig(cfg InstallConfig) {
func NewInstall(ai *action.Install) *install {
return &install{ai}
}
type MockInstall struct{ mock.Mock }
func (m *MockInstall) SetConfig(cfg InstallConfig) {
m.Called(cfg)
}
func (m *MockInstall) Run(c *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
args := m.Called(c, vals)
return args.Get(0).(*release.Release), args.Error(1)
}

@ -7,15 +7,16 @@ import (
)
type ListRequest struct {
RequestID string
NameSpace string `json:"namespace"`
ReleaseStatus string `json:"release_status"`
}
type ListResponse struct {
Error string `json:"error,omitempty"`
Data []HelmRelease
Data []Releases
}
type HelmRelease struct {
type Releases struct {
Release string `json:"release"`
Namespace string `json:"namespace"`
}
@ -27,7 +28,6 @@ func List(svc Service) http.Handler {
var response ListResponse
var request ListRequest
decoder := json.NewDecoder(r.Body)
decoder.UseNumber()
if err := decoder.Decode(&request); err != nil {
logger.Errorf("[List] error decoding request: %v", err)
response.Error = err.Error()
@ -38,10 +38,7 @@ func List(svc Service) http.Handler {
}
defer r.Body.Close()
request.RequestID = r.Header.Get("Request-Id")
svc.lister.SetStateMask()
res, err := svc.lister.Run()
helmReleases, err := svc.List(request.ReleaseStatus)
if err != nil {
logger.Errorf("[List] error while installing chart: %v", err)
@ -52,11 +49,6 @@ func List(svc Service) http.Handler {
return
}
var helmReleases []HelmRelease
for _, eachRes := range res {
r := HelmRelease{Release: eachRes.Name, Namespace: eachRes.Namespace}
helmReleases = append(helmReleases, r)
}
response = ListResponse{"", helmReleases}
payload, err := json.Marshal(response)
if err != nil {

@ -2,9 +2,11 @@ package api_test
import (
"fmt"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/stretchr/testify/suite"
"gotest.tools/assert"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/api"
"helm.sh/helm/v3/pkg/api/logger"
"helm.sh/helm/v3/pkg/cli"
@ -20,21 +22,36 @@ type ListTestSuite struct {
suite.Suite
recorder *httptest.ResponseRecorder
server *httptest.Server
mockInstall *api.MockInstall
mockChartLoader *api.MockChartLoader
mockList *api.MockList
mockInstall *mockInstall
mockChartLoader *mockChartLoader
mockList *mockList
appConfig *cli.EnvSettings
}
type mockList struct{ mock.Mock }
func (m *mockList) Run() ([]*release.Release, error) {
args := m.Called()
return args.Get(0).([]*release.Release), args.Error(1)
}
func (m *mockList) SetStateMask() {
m.Called()
}
func (m *mockList) SetState(state action.ListStates) {
m.Called(state)
}
func (s *ListTestSuite) SetupSuite() {
logger.Setup("default")
}
func (s *ListTestSuite) SetupTest() {
s.recorder = httptest.NewRecorder()
s.mockInstall = new(api.MockInstall)
s.mockChartLoader = new(api.MockChartLoader)
s.mockList = new(api.MockList)
s.mockInstall = new(mockInstall)
s.mockChartLoader = new(mockChartLoader)
s.mockList = new(mockList)
s.appConfig = &cli.EnvSettings{
RepositoryConfig: "./testdata/helm",
PluginsDirectory: "./testdata/helm/plugin",
@ -45,7 +62,7 @@ func (s *ListTestSuite) SetupTest() {
}
func (s *ListTestSuite) TestShouldReturnReleasesWhenSuccessfulAPICall() {
body := `{"request_id":"test-request-id"}`
body := `{"release_status":"deployed"}`
req, _ := http.NewRequest("POST", fmt.Sprintf("%s/list", s.server.URL), strings.NewReader(body))
var releases []*release.Release
@ -55,6 +72,7 @@ func (s *ListTestSuite) TestShouldReturnReleasesWhenSuccessfulAPICall() {
Info: &release.Info{Status: release.StatusDeployed}})
s.mockList.On("SetStateMask")
s.mockList.On("SetState", action.ListDeployed)
s.mockList.On("Run").Return(releases, nil)
resp, err := http.DefaultClient.Do(req)

@ -1,7 +1,6 @@
package api
import (
"github.com/stretchr/testify/mock"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/release"
)
@ -13,20 +12,13 @@ type list struct {
type lister interface {
Run() ([]*release.Release, error)
SetStateMask()
SetState(state action.ListStates)
}
func NewList(action *action.List) *list{
return &list{action}
}
type MockList struct{ mock.Mock }
func (m *MockList) Run() ([]*release.Release, error) {
args := m.Called()
return args.Get(0).([]*release.Release), args.Error(1)
func (l *list) SetState(state action.ListStates){
l.StateMask = state
}
func (m *MockList) SetStateMask() {
m.Called()
}

@ -3,6 +3,7 @@ package api
import (
"context"
"fmt"
"helm.sh/helm/v3/pkg/action"
"helm.sh/helm/v3/pkg/api/logger"
"helm.sh/helm/v3/pkg/chart"
@ -24,13 +25,13 @@ type InstallConfig struct {
ChartName string
}
type chartValues map[string]interface{}
type ChartValues map[string]interface{}
type installResult struct {
status string
Status string
}
func (s Service) getValues(vals chartValues) (chartValues, error) {
func (s Service) getValues(vals ChartValues) (ChartValues, error) {
// valueOpts := &values.Options{}
//valueOpts.Values = append(valueOpts.Values, vals)
//TODO: we need to make this as Provider, so it'll be able to merge
@ -38,7 +39,7 @@ func (s Service) getValues(vals chartValues) (chartValues, error) {
return vals, nil
}
func (s Service) Install(ctx context.Context, cfg InstallConfig, values chartValues) (*installResult, error) {
func (s Service) Install(ctx context.Context, cfg InstallConfig, values ChartValues) (*installResult, error) {
chart, err := s.loadChart(cfg.ChartName)
if err != nil {
return nil, err
@ -63,7 +64,7 @@ func (s Service) loadChart(chartName string) (*chart.Chart, error) {
return requestedChart, nil
}
func (s Service) installChart(icfg InstallConfig, ch *chart.Chart, vals chartValues) (*installResult, error) {
func (s Service) installChart(icfg InstallConfig, ch *chart.Chart, vals ChartValues) (*installResult, error) {
s.Installer.SetConfig(icfg)
release, err := s.Installer.Run(ch, vals)
if err != nil {
@ -71,11 +72,29 @@ func (s Service) installChart(icfg InstallConfig, ch *chart.Chart, vals chartVal
}
result := new(installResult)
if release.Info != nil {
result.status = release.Info.Status.String()
result.Status = release.Info.Status.String()
}
return result, nil
}
func (s Service) List(releaseStatus string) ([]Releases, error) {
listStates := new(action.ListStates)
s.lister.SetState(listStates.FromName(releaseStatus))
s.lister.SetStateMask()
releases, err := s.lister.Run()
if err != nil {
return nil, err
}
var helmReleases []Releases
for _, eachRes := range releases {
r := Releases{Release: eachRes.Name, Namespace: eachRes.Namespace}
helmReleases = append(helmReleases, r)
}
return helmReleases, nil
}
func NewService(settings *cli.EnvSettings, cl chartloader, i Installer, l lister) Service {
return Service{
settings,

@ -1,8 +1,9 @@
package api
package api_test
import (
"context"
"errors"
"helm.sh/helm/v3/pkg/api"
"testing"
"github.com/stretchr/testify/assert"
@ -17,31 +18,31 @@ import (
type ServiceTestSuite struct {
suite.Suite
ctx context.Context
installer *MockInstall
chartloader *MockChartLoader
lister *MockList
svc Service
installer *mockInstall
chartloader *mockChartLoader
lister *mockList
svc api.Service
settings *cli.EnvSettings
}
func (s *ServiceTestSuite) SetupTest() {
logger.Setup("")
s.ctx = context.Background()
s.installer = new(MockInstall)
s.lister = new(MockList)
s.chartloader = new(MockChartLoader)
s.installer = new(mockInstall)
s.lister = new(mockList)
s.chartloader = new(mockChartLoader)
s.settings = &cli.EnvSettings{}
s.svc = NewService(s.settings, s.chartloader, s.installer, s.lister)
s.svc = api.NewService(s.settings, s.chartloader, s.installer, s.lister)
}
func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnInvalidChart() {
chartName := "stable/invalid-chart"
cfg := InstallConfig{
cfg := api.InstallConfig{
Name: "some-component",
Namespace: "hermes",
ChartName: chartName,
}
var vals chartValues
var vals api.ChartValues
s.chartloader.On("LocateChart", chartName, s.settings).Return("", errors.New("Unable to find chart"))
res, err := s.svc.Install(s.ctx, cfg, vals)
@ -56,7 +57,7 @@ func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnInvalidChart() {
func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnFailedIntallRun() {
chartName := "stable/valid-chart"
cfg := InstallConfig{
cfg := api.InstallConfig{
Name: "some-component",
Namespace: "hermes",
ChartName: chartName,
@ -78,7 +79,7 @@ func (s *ServiceTestSuite) TestInstallShouldReturnErrorOnFailedIntallRun() {
func (s *ServiceTestSuite) TestInstallShouldReturnResultOnSuccess() {
chartName := "stable/valid-chart"
cfg := InstallConfig{
cfg := api.InstallConfig{
Name: "some-component",
Namespace: "hermes",
ChartName: chartName,
@ -94,7 +95,7 @@ func (s *ServiceTestSuite) TestInstallShouldReturnResultOnSuccess() {
t := s.T()
assert.NoError(t, err)
require.NotNil(t, res)
assert.Equal(t, res.status, "deployed")
assert.Equal(t, res.Status, "deployed")
s.chartloader.AssertExpectations(t)
s.installer.AssertExpectations(t)
}

@ -99,7 +99,7 @@ func (h UpgradeHandler) UpgradeRelease(releaseName, releaseNamespace, chartPath
fmt.Printf("error in request: %v", err)
return false, "", err
}
return true, release.status, nil
return true, release.Status, nil
}
}

Loading…
Cancel
Save