Skip to content

Commit

Permalink
Surface activestate.yaml conditional errors as user input errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
mitchell-as committed May 30, 2024
1 parent 277eb20 commit 9a15cb0
Show file tree
Hide file tree
Showing 15 changed files with 110 additions and 48 deletions.
2 changes: 1 addition & 1 deletion internal/captain/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ func (c *Command) cobraExecHandler(cobraCmd *cobra.Command, args []string) (rerr
}

if err := c.execute(c, args); err != nil {
if !locale.IsError(err) {
if !locale.HasError(err) {
return locale.WrapError(err, "unexpected_error", "Command failed due to unexpected error. For your convenience, this is the error chain:\n{{.V0}}", errs.JoinMessage(err))
}
return errs.Wrap(err, "execute failed")
Expand Down
4 changes: 2 additions & 2 deletions internal/constraints/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (c *Conditional) RegisterParam(name string, value interface{}) {
}

func (c *Conditional) Eval(conditional string) (bool, error) {
tpl, err := template.New("letter").Funcs(c.funcs).Parse(fmt.Sprintf(`{{if %s}}1{{end}}`, conditional))
tpl, err := template.New("").Funcs(c.funcs).Parse(fmt.Sprintf(`{{if %s}}1{{end}}`, conditional))
if err != nil {
return false, locale.WrapInputError(err, "err_conditional", "Invalid 'if' condition: '{{.V0}}', error: '{{.V1}}'.", conditional, err.Error())
}
Expand Down Expand Up @@ -154,7 +154,7 @@ func FilterUnconstrained(conditional *Conditional, items []projectfile.Constrain
if conditional != nil && item.ConditionalFilter() != "" {
isTrue, err := conditional.Eval(string(item.ConditionalFilter()))
if err != nil {
return nil, err
return nil, locale.WrapInputError(err, "err_conditional_eval", "There was an error with the conditional in your activestate.yaml's '{{.V0}}' script", item.ID())
}

if isTrue {
Expand Down
7 changes: 6 additions & 1 deletion internal/events/cmdcall/cmdcall.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmdcall
import (
"strings"

"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
Expand Down Expand Up @@ -100,7 +101,11 @@ func (cc *CmdCall) Run(eventType project.EventType) error {
}

scriptName, scriptArgs := ss[0], ss[1:]
if err := cc.scriptrun.Run(cc.proj.ScriptByName(scriptName), scriptArgs); err != nil {
script, err := cc.proj.ScriptByName(scriptName)
if err != nil {
return errs.Wrap(err, "Could not get script")
}
if err := cc.scriptrun.Run(script, scriptArgs); err != nil {
return locale.WrapError(
err, "cmdcall_event_err_script_run",
"Failure running defined script '[NOTICE]{{.V0}}[/RESET]' for event '[NOTICE]{{.V1}}[/RESET]'",
Expand Down
7 changes: 6 additions & 1 deletion internal/runners/export/ghactions/ghactions.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package ghactions
import (
"gopkg.in/yaml.v2"

"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/primer"
Expand Down Expand Up @@ -61,7 +62,11 @@ func (g *GithubActions) Run(p *Params) error {
}
workflowJob.Env[constant.Name()] = v
}
for _, script := range job.Scripts() {
scripts, err := job.Scripts()
if err != nil {
return errs.Wrap(err, "Could not get scripts")
}
for _, script := range scripts {
workflowJob.Steps = append(workflowJob.Steps, WorkflowStep{
Name: script.Name(),
Run: "state run " + script.Name(),
Expand Down
6 changes: 5 additions & 1 deletion internal/runners/run/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/ActiveState/cli/internal/analytics"
"github.com/ActiveState/cli/internal/config"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/language"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
Expand Down Expand Up @@ -73,7 +74,10 @@ func (r *Run) Run(name string, args []string) error {
checker.RunCommitsBehindNotifier(r.proj, r.out, r.auth)
}

script := r.proj.ScriptByName(name)
script, err := r.proj.ScriptByName(name)
if err != nil {
return errs.Wrap(err, "Could not get script")
}
if script == nil {
return locale.NewInputError("error_state_run_unknown_name", "", name)
}
Expand Down
12 changes: 9 additions & 3 deletions internal/runners/scripts/edit.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,15 @@ func (e *Edit) Run(params *EditParams) error {
return rationalize.ErrNoProject
}

script := e.project.ScriptByName(params.Name)
script, err := e.project.ScriptByName(params.Name)
if err != nil {
return errs.Wrap(err, "Could not get script")
}
if script == nil {
return locale.NewInputError("edit_scripts_no_name", "Could not find script with the given name {{.V0}}", params.Name)
}

err := e.editScript(script, params)
err = e.editScript(script, params)
if err != nil {
return locale.WrapError(err, "error_edit_script", "Failed to edit script.")
}
Expand Down Expand Up @@ -259,7 +262,10 @@ func updateProjectFile(cfg projectfile.ConfigGetter, pj *project.Project, script
}

pjf := pj.Source()
script := pj.ScriptByName(name)
script, err := pj.ScriptByName(name)
if err != nil {
return errs.Wrap(err, "Could not get script")
}
if script == nil {
return locale.NewError("err_update_script_cannot_find", "Could not find the source script to update.")
}
Expand Down
20 changes: 11 additions & 9 deletions internal/runners/scripts/edit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ func (suite *EditTestSuite) AfterTest(suiteName, testName string) {
}

func (suite *EditTestSuite) TestCreateScriptFile() {
script := suite.project.ScriptByName("hello")
script, err := suite.project.ScriptByName("hello")
suite.Require().NoError(err)

var err error
suite.scriptFile, err = createScriptFile(script, false)
suite.Require().NoError(err, "should create file")
}

func (suite *EditTestSuite) TestCreateScriptFile_Expand() {
script := suite.project.ScriptByName("hello-constant")
script, err := suite.project.ScriptByName("hello-constant")
suite.Require().NoError(err)

var err error
suite.scriptFile, err = createScriptFile(script, true)
suite.Require().NoError(err, "should create file")

Expand All @@ -98,9 +98,9 @@ func (suite *EditTestSuite) TestCreateScriptFile_Expand() {
}

func (suite *EditTestSuite) TestNewScriptWatcher() {
script := suite.project.ScriptByName("hello")
script, err := suite.project.ScriptByName("hello")
suite.Require().NoError(err)

var err error
suite.scriptFile, err = createScriptFile(script, false)
suite.Require().NoError(err, "should create file")

Expand All @@ -123,9 +123,9 @@ func (suite *EditTestSuite) TestNewScriptWatcher() {
}

func (suite *EditTestSuite) TestUpdateProjectFile() {
replace := suite.project.ScriptByName("replace")
replace, err := suite.project.ScriptByName("replace")
suite.Require().NoError(err)

var err error
suite.scriptFile, err = createScriptFile(replace, false)
suite.Require().NoError(err, "unexpected error creating script file")

Expand All @@ -138,7 +138,9 @@ func (suite *EditTestSuite) TestUpdateProjectFile() {
suite.Require().NoError(err, "unexpected error getting project")
v1, err := replace.Value()
suite.Require().NoError(err)
v2, err := updatedProject.ScriptByName("replace").Value()
script, err := updatedProject.ScriptByName("replace")
suite.Require().NoError(err)
v2, err := script.Value()
suite.Require().NoError(err)
suite.Equal(v1, v2)
}
Expand Down
9 changes: 7 additions & 2 deletions internal/runners/scripts/scripts.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package scripts

import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
Expand Down Expand Up @@ -44,8 +45,12 @@ func (s *Scripts) Run() error {
name, owner := s.project.Name(), s.project.Owner()
logging.Debug("listing scripts for org=%s, project=%s", owner, name)

scripts := make([]scriptLine, len(s.project.Scripts()))
for i, s := range s.project.Scripts() {
projectScripts, err := s.project.Scripts()
if err != nil {
return errs.Wrap(err, "Could not get scripts")
}
scripts := make([]scriptLine, len(projectScripts))
for i, s := range projectScripts {
scripts[i] = scriptLine{s.Name(), s.Description()}
}

Expand Down
31 changes: 22 additions & 9 deletions internal/scriptrun/test/integration/scriptrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,9 @@ scripts:
require.NoError(t, err)
defer func() { require.NoError(t, cfg.Close()) }()
scriptRun := scriptrun.New(auth, outputhelper.NewCatcher(), subshell.New(cfg), proj, cfg, blackhole.New(), nil)
err = scriptRun.Run(proj.ScriptByName("run"), []string{})
script, err := proj.ScriptByName("run")
require.NoError(t, err)
err = scriptRun.Run(script, []string{})
assert.NoError(t, err, "No error occurred")
}

Expand Down Expand Up @@ -119,7 +121,9 @@ func (suite *ScriptRunSuite) TestEnvIsSet() {

out := capturer.CaptureOutput(func() {
scriptRun := scriptrun.New(auth, outputhelper.NewCatcher(), subshell.New(cfg), proj, cfg, blackhole.New(), nil)
err = scriptRun.Run(proj.ScriptByName("run"), nil)
script, err := proj.ScriptByName("run")
require.NoError(t, err, "Error: "+errs.JoinMessage(err))
err = scriptRun.Run(script, nil)
assert.NoError(t, err, "Error: "+errs.JoinMessage(err))
})

Expand Down Expand Up @@ -166,8 +170,10 @@ scripts:

out := outputhelper.NewCatcher()
scriptRun := scriptrun.New(auth, out, subshell.New(cfg), proj, cfg, blackhole.New(), nil)
fmt.Println(proj.ScriptByName("run"))
err = scriptRun.Run(proj.ScriptByName("run"), nil)
script, err := proj.ScriptByName("run")
fmt.Println(script)
require.NoError(t, err)
err = scriptRun.Run(script, nil)
assert.NoError(t, err, "No error occurred")
}

Expand Down Expand Up @@ -198,7 +204,7 @@ scripts:

scriptRun := scriptrun.New(auth, outputhelper.NewCatcher(), subshell.New(cfg), proj, cfg, blackhole.New(), nil)
err = scriptRun.Run(nil, nil)
assert.Error(t, err, "Error occurred")
assert.Error(t, err, "No error occurred")
}

func (suite *ScriptRunSuite) TestRunUnknownCommand() {
Expand Down Expand Up @@ -228,8 +234,10 @@ scripts:
defer func() { require.NoError(t, cfg.Close()) }()

scriptRun := scriptrun.New(auth, outputhelper.NewCatcher(), subshell.New(cfg), proj, cfg, blackhole.New(), nil)
err = scriptRun.Run(proj.ScriptByName("run"), nil)
assert.Error(t, err, "Error occurred")
script, err := proj.ScriptByName("run")
require.NoError(t, err)
err = scriptRun.Run(script, nil)
assert.Error(t, err, "No error occurred")
}

func (suite *ScriptRunSuite) TestRunActivatedCommand() {
Expand Down Expand Up @@ -282,7 +290,9 @@ scripts:

// Run the command.
scriptRun := scriptrun.New(auth, outputhelper.NewCatcher(), subshell.New(cfg), proj, cfg, blackhole.New(), nil)
err = scriptRun.Run(proj.ScriptByName("run"), nil)
script, err := proj.ScriptByName("run")
require.NoError(t, err)
err = scriptRun.Run(script, nil)
assert.NoError(t, err, "No error occurred")

// Reset.
Expand Down Expand Up @@ -378,7 +388,10 @@ func captureExecCommand(t *testing.T, tmplCmdName, cmdName string, cmdArgs []str

outStr, outErr := osutil.CaptureStdout(func() {
scriptRun := scriptrun.New(auth, outputhelper.NewCatcher(), subshell.New(cfg), proj, cfg, blackhole.New(), nil)
err = scriptRun.Run(proj.ScriptByName(cmdName), cmdArgs)
var script *project.Script
if script, err = proj.ScriptByName(cmdName); err == nil {
err = scriptRun.Run(script, cmdArgs)
}
})
require.NoError(t, outErr, "error capturing stdout")

Expand Down
6 changes: 5 additions & 1 deletion internal/subshell/sscommon/rcfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,11 @@ func SetupProjectRcFile(prj *project.Project, templateName, ext string, env map[
globalBinDir := filepath.Clean(storage.GlobalBinDir())

// Prepare script map to be parsed by template
for _, cmd := range prj.Scripts() {
projectScripts, err := prj.Scripts()
if err != nil {
return nil, errs.Wrap(err, "Could not get project scripts")
}
for _, cmd := range projectScripts {
explicitName = fmt.Sprintf("%s_%s", prj.NormalizedName(), cmd.Name())

path, err := exec.LookPath(cmd.Name())
Expand Down
5 changes: 4 additions & 1 deletion pkg/project/expander.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ func EventExpander(_ string, name string, meta string, isFunction bool, ctx *Exp

// ScriptExpander expands scripts defined in the project-file.
func ScriptExpander(_ string, name string, meta string, isFunction bool, ctx *Expansion) (string, error) {
script := ctx.Project.ScriptByName(name)
script, err := ctx.Project.ScriptByName(name)
if err != nil {
return "", errs.Wrap(err, "Could not get script")
}
if script == nil {
return "", nil
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/project/expander_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,8 @@ func TestExpandScriptPathRecursive(t *testing.T) {

func TestExpandBashScriptPath(t *testing.T) {
prj := loadProject(t)
script := prj.ScriptByName("bashScriptPath")
script, err := prj.ScriptByName("bashScriptPath")
require.NoError(t, err)
require.NotNil(t, script, "bashScriptPath script does not exist")
value, err := script.Value()
require.NoError(t, err)
Expand Down
28 changes: 18 additions & 10 deletions pkg/project/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,27 +141,31 @@ func (p *Project) EventByName(name string, bashifyPaths bool) *Event {
}

// Scripts returns a reference to projectfile.Scripts
func (p *Project) Scripts() []*Script {
func (p *Project) Scripts() ([]*Script, error) {
constrained, err := constraints.FilterUnconstrained(pConditional, p.projectfile.Scripts.AsConstrainedEntities())
if err != nil {
logging.Warning("Could not filter unconstrained scripts: %v", err)
return nil, errs.Wrap(err, "Could not filter unconstrained scripts")
}
scs := projectfile.MakeScriptsFromConstrainedEntities(constrained)
scripts := make([]*Script, 0, len(scs))
for _, s := range scs {
scripts = append(scripts, &Script{s, p})
}
return scripts
return scripts, nil
}

// ScriptByName returns a reference to a projectfile.Script with a given name.
func (p *Project) ScriptByName(name string) *Script {
for _, script := range p.Scripts() {
func (p *Project) ScriptByName(name string) (*Script, error) {
scripts, err := p.Scripts()
if err != nil {
return nil, errs.Wrap(err, "Could not get scripts")
}
for _, script := range scripts {
if script.Name() == name {
return script
return script, nil
}
}
return nil
return nil, nil
}

// Jobs returns a reference to projectfile.Jobs
Expand Down Expand Up @@ -589,12 +593,16 @@ func (j *Job) Constants() []*Constant {
return constants
}

func (j *Job) Scripts() []*Script {
func (j *Job) Scripts() ([]*Script, error) {
scripts := []*Script{}
for _, scriptName := range j.job.Scripts {
if script := j.project.ScriptByName(scriptName); script != nil {
script, err := j.project.ScriptByName(scriptName)
if err != nil {
return nil, errs.Wrap(err, "Could not get script")
}
if script != nil {
scripts = append(scripts, script)
}
}
return scripts
return scripts, nil
}
Loading

0 comments on commit 9a15cb0

Please sign in to comment.