Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Fix- Print error messages in pre builder #829

Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
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: 36 additions & 0 deletions internal/mocks/formula.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2020 ZUP IT SERVICOS EM TECNOLOGIA E INOVACAO SA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package mocks

import (
"github.com/ZupIT/ritchie-cli/pkg/formula"
"github.com/stretchr/testify/mock"
)

type BuilderMock struct {
mock.Mock
}

func (b *BuilderMock) Build(info formula.BuildInfo) error {
args := b.Called(info)
return args.Error(0)
}

func (b *BuilderMock) HasBuilt() bool {
args := b.Called()
return args.Bool(0)
}
28 changes: 28 additions & 0 deletions internal/mocks/runner.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright 2020 ZUP IT SERVICOS EM TECNOLOGIA E INOVACAO SA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package mocks

import "github.com/stretchr/testify/mock"

type PreRunBuilder struct {
mock.Mock
}

func (prb *PreRunBuilder) Build(relativePath string) error {
args := prb.Called(relativePath)
return args.Error(0)
}
46 changes: 46 additions & 0 deletions internal/mocks/workspace.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* Copyright 2020 ZUP IT SERVICOS EM TECNOLOGIA E INOVACAO SA
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package mocks

import (
"github.com/ZupIT/ritchie-cli/pkg/formula"
"github.com/stretchr/testify/mock"
)

type WorkspaceListHasherMock struct {
mock.Mock
}

func (w *WorkspaceListHasherMock) List() (formula.Workspaces, error) {
args := w.Called()
return args.Get(0).(formula.Workspaces), args.Error(1)
}

func (w *WorkspaceListHasherMock) CurrentHash(formulaPath string) (string, error) {
args := w.Called(formulaPath)
return args.String(0), args.Error(1)
}

func (w *WorkspaceListHasherMock) PreviousHash(formulaPath string) (string, error) {
args := w.Called(formulaPath)
return args.String(0), args.Error(1)
}

func (w *WorkspaceListHasherMock) UpdateHash(formulaPath string, hash string) error {
args := w.Called(formulaPath, hash)
return args.Error(0)
}
2 changes: 2 additions & 0 deletions pkg/cmd/add_repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"reflect"
"sort"
"strings"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -287,6 +288,7 @@ func (ar *addRepoCmd) resolveFlags(cmd *cobra.Command) (formula.Repo, error) {
}
}
if !providerValid {
sort.Strings(providers)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move the sort to inside the ar.repoProviders.List() method? It should not be the responsibility of this command to know that. Also, this way we only need to edit a single point in code, other packages invoking this method could stumble upon the same problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I can change, you're right.

return formula.Repo{}, errors.New("please select a provider from " + strings.Join(providers, ", "))
}

Expand Down
6 changes: 5 additions & 1 deletion pkg/cmd/add_repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cmd

import (
"errors"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -45,6 +46,9 @@ func TestAddRepoCmd(t *testing.T) {
repoProviders.Add("GitLab", formula.Git{Repos: gitRepositoryWithoutTagsMock, NewRepoInfo: github.NewRepoInfo})
repoProviders.Add("Bitbucket", formula.Git{Repos: gitRepositoryErrorsMock, NewRepoInfo: github.NewRepoInfo})

repoProvidersSorted := repoProviders.List()
sort.Strings(repoProvidersSorted)

repoTest := &formula.Repo{
Provider: "Github",
Name: "someRepo1",
Expand Down Expand Up @@ -247,7 +251,7 @@ func TestAddRepoCmd(t *testing.T) {
name: "fail flags with wrong provider",
args: []string{"--provider=github"},
fields: fields{},
want: errors.New("please select a provider from " + strings.Join(repoProviders.List(), ", ")),
want: errors.New("please select a provider from " + strings.Join(repoProvidersSorted, ", ")),
},
{
name: "fail flags with empty name",
Expand Down
2 changes: 1 addition & 1 deletion pkg/formula/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ type Executor interface {
}

type PreRunBuilder interface {
Build(string)
Build(string) error
}

type PreRunner interface {
Expand Down
4 changes: 3 additions & 1 deletion pkg/formula/runner/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ func (ex ExecutorManager) Execute(exe formula.ExecuteData) error {
}

if strings.HasPrefix(exe.Def.RepoName, "local-") {
ex.preRunBuilder.Build(exe.Def.Path)
if err := ex.preRunBuilder.Build(exe.Def.Path); err != nil {
return err
}
}

if err := runner.Run(exe.Def, exe.InType, exe.Verbose, exe.Flags); err != nil {
Expand Down
58 changes: 47 additions & 11 deletions pkg/formula/runner/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,16 +21,20 @@ import (
"testing"

"github.com/spf13/pflag"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"

"github.com/ZupIT/ritchie-cli/internal/mocks"
"github.com/ZupIT/ritchie-cli/pkg/api"
"github.com/ZupIT/ritchie-cli/pkg/formula"
)

func TestExecute(t *testing.T) {
type in struct {
runners formula.Runners
config formula.ConfigRunner
exe formula.ExecuteData
runners formula.Runners
config formula.ConfigRunner
exe formula.ExecuteData
preRunBuilderError error
}

tests := []struct {
Expand Down Expand Up @@ -89,6 +93,23 @@ func TestExecute(t *testing.T) {
},
want: nil,
},
{
name: "execute repo local success",
in: in{
runners: formula.Runners{
formula.LocalRun: localRunnerMock{},
formula.DockerRun: dockerRunnerMock{},
},
config: configRunnerMock{runType: formula.LocalRun},
exe: formula.ExecuteData{
Def: formula.Definition{RepoName: "local-user"},
InType: 0,
RunType: formula.DefaultRun,
Verbose: false,
},
},
want: nil,
},
{
name: "find default runner error",
in: in{
Expand Down Expand Up @@ -123,16 +144,35 @@ func TestExecute(t *testing.T) {
},
want: errors.New("error to run formula"),
},
{
name: "run pre run builder error",
in: in{
runners: formula.Runners{
formula.LocalRun: localRunnerMock{},
formula.DockerRun: dockerRunnerMock{},
},
config: configRunnerMock{runType: formula.DockerRun},
exe: formula.ExecuteData{
Def: formula.Definition{RepoName: "local-teste"},
InType: 0,
RunType: formula.DefaultRun,
Verbose: false,
},
preRunBuilderError: errors.New("error to pre run builder formula"),
},
want: errors.New("error to pre run builder formula"),
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
executorManager := NewExecutor(tt.in.runners, preRunBuilderMock{}, tt.in.config)
preRunBuilder := new(mocks.PreRunBuilder)
preRunBuilder.On("Build", mock.Anything).Return(tt.in.preRunBuilderError)

executorManager := NewExecutor(tt.in.runners, preRunBuilder, tt.in.config)
got := executorManager.Execute(tt.in.exe)

if (tt.want != nil && got == nil) || got != nil && got.Error() != tt.want.Error() {
t.Errorf("Execute(%s) got %v, want %v", tt.name, got, tt.want)
}
assert.Equal(t, tt.want, got)
})
}
}
Expand Down Expand Up @@ -166,7 +206,3 @@ func (c configRunnerMock) Create(runType formula.RunnerType) error {
func (c configRunnerMock) Find() (formula.RunnerType, error) {
return c.runType, c.findErr
}

type preRunBuilderMock struct{}

func (bm preRunBuilderMock) Build(string) {}
20 changes: 11 additions & 9 deletions pkg/formula/runner/pre_run_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
package runner

import (
"errors"
"fmt"
"path/filepath"

"github.com/ZupIT/ritchie-cli/pkg/formula"
"github.com/ZupIT/ritchie-cli/pkg/prompt"
)

const (
messageChangeError = "Failed to detect formula changes, executing the last build"
messageBuildError = "Failed to build formula"
messageChangeError = "Failed to detect formula changes, executing the last build: %s"
messageBuildError = "Failed to build formula: %s"
)

type PreRunBuilderManager struct {
Expand All @@ -44,22 +44,24 @@ func NewPreRunBuilder(
}
}

func (b PreRunBuilderManager) Build(relativePath string) {
func (b PreRunBuilderManager) Build(relativePath string) error {
workspace, err := b.modifiedWorkspace(relativePath)
if err != nil {
fmt.Println(prompt.Yellow(messageChangeError))
return
msg := fmt.Sprintf(messageChangeError, err.Error())
return errors.New(msg)
}

// No modifications on any workspace, skip
if workspace == nil {
return
return nil
}

if err = b.buildOnWorkspace(*workspace, relativePath); err != nil {
fmt.Println(prompt.Red(messageBuildError))
return
msg := fmt.Sprintf(messageBuildError, err.Error())
return errors.New(msg)
}

return nil
}

func (b PreRunBuilderManager) modifiedWorkspace(relativePath string) (*formula.Workspace, error) {
Expand Down
Loading