--dir bug

[billychow68]

Prerequisites

• Can you reproduce the problem?
• Are you running the latest version?
• Did you perform a cursory search?

Description

--dir flag doesn't work with commands with shell scripts

Steps to Reproduce

1. Unpack the attachment
2. Execute
   ./commander test tests/test.yaml
   Expected behavior: the test should pass
   Actual behavior: the test passed
3. Execute
   ./commander test --dir tests/
   Expected behavior: the test should pass
   Actual behavior:

Starting test against directory: tests/...

panic: yaml: unmarshal errors:
  line 3: cannot unmarshal !!str `echo "H...` into struct { Tests map[string]suite.YAMLTest "yaml:\"tests\""; Config suite.YAMLTestConfigConf "yaml:\"config\""; Nodes map[string]suite.YAMLNodeConf "yaml:\"nodes\"" }

goroutine 18 [running]:
github.com/SimonBaeumer/commander/pkg/suite.ParseYAML(0xc00032e000, 0x1a, 0x21a, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
	/home/travis/gopath/src/github.com/SimonBaeumer/commander/pkg/suite/yaml_suite.go:57 +0x2cc
github.com/SimonBaeumer/commander/pkg/app.testFile(0xc0000260c0, 0xd, 0x0, 0x0, 0xd, 0x0, 0x0)
	/home/travis/gopath/src/github.com/SimonBaeumer/commander/pkg/app/test_command.go:102 +0xa7
github.com/SimonBaeumer/commander/pkg/app.testDir.func1(0xc0002e0c00, 0xc000215fc0, 0x2, 0x2, 0x7ffeefbff97f, 0x6, 0xc000080960)
	/home/travis/gopath/src/github.com/SimonBaeumer/commander/pkg/app/test_command.go:75 +0x19b
created by github.com/SimonBaeumer/commander/pkg/app.testDir
	/home/travis/gopath/src/github.com/SimonBaeumer/commander/pkg/app/test_command.go:67 +0x182

dir-bug.zip

Specifications

• Version: 2.1.0
• Platform: Darwin
• Subsystem: ?

[dylanhitt]

Hey @billychow68, good timing on this.

Currently if you have non test suite files (in your case .sh) in the same directory that you're testing, commander will panic. If you remove the shell script and change your pathing it will work. I think we have two options:

1. Simply display a better error describing that you can not have non test-suite files in the dir you are testing.
2. Skip over files that are not valid suite files. So currently skip everything except yaml. Now this leads to some other issues, let's say we have a k8s config in our dir. In this case a warning may be nice displaying Warn: invalid yaml config <filename>

Thank you again, if you have any thoughts please share them.

[billychow68]

Thanks @dylanhitt for the quick response. In my case, I think as the number of test suites and shell scripts increases it makes sense to organize them better by separating them. I've tried the following structures and both worked perfectly!

$ tree
.
├── commander
├── run.sh
└── tests
    ├── scripts
    │   └── test.sh
    └── test.yaml

$ tree
.
├── commander
├── run.sh
├── scripts
│   └── test.sh
└── tests
    └── test.yaml

[SimonBaeumer]

Can we improve something, i.e. documentation to the test help message or can I close it?

[dylanhitt]

Hey @SimonBaeumer,

I thought about this more. After #121 is closed I'll make a PR for the desired long term fix. I realized today that in order validate the entire dir commander must loop over all objects in the dir and verify them before we start executing. I was debating on adding a Warnings to the output of the Summary of results for valid commander markup files as it would be nice to provide the user the flexibility of having whatever they wants in in their test dir. As well as notifying the user of possible invalid configs, this is a special case for dir as file will still display an error.

I will open a PR for the help message for now.

[SimonBaeumer]

The documentation does not say too much about the actual limitations.
It is possible to store other files into the directory in which our test suites are located.
The limitation is that the current working directory (cwd) is always the location where commander was executed.

Atm commander does only work on pathes relative to the cwd of the commander process.
The solution imho is too provide a way for passing an asset directory or target dir where it can find the scripts to execute.

├── bin
│   └── test.exe
├── commander
├── scripts
│   └── random-script.sh
└── tests
    └── test01-commander.yaml

The test file would look something like this, that it is enforced to always provide the complete path, realtive to the commander execution:

tests:
    bin/test.exe:
       exit-code: ....
    scripts/random-script.sh:
       exit-code: .... 

A solution would be to execute the scripts relative to the yaml file or proividing a config which allows to configure the base directory, i.e.

config:
   base-dir: bin
tests:
    test.exe:
       exit-code: ....
    random-script.sh:
       exit-code: .... 
       config:
           base-dir: bin

Additionally a template variable like {{ .yaml-dir }} allows to set the directory relative to the path of the test file.
This would make the execution of the suites independent of the commander process.

Thoughts on this approach @dylanhitt?

[dylanhitt]

It is possible to store other files into the directory in which our test suites are located.

Currently when executing with --dir any file that is not a valid config throws this error. I opened #131 to address this issue. I opened PR #132 to address this issue for the short term.

In regards to providing a config for users to configure their asset directories I personally like the config route, essentially the prototype you provided. I personally like that it is more explicit and would provide plenty of flexibility for both relative and absolute pathing. We may want to open another issue to address this specifically.

[SimonBaeumer]

Currently when executing with --dir any file that is not a valid config throws this error. I opened #131 to address this issue. I opened PR #132 to address this issue for the short term.

Where does this happen? I tried it yesterday and did not get any error if I store other files into the directory tree.

[dylanhitt]

@billychow68 provided a pretty nice zip file for this. I think you're storing files into a subdir which will currently be skipped. Take a look at this line. To summarize if you're testing a directory with any invalid file extension or invalid config in the same directory being tested we currently just panic. My thoughts on how to handle this issue are expressed in #131, as we need to validate the entire dir before exec. To see exactly what I mean pull down the zip file and give it a shot with v2.1.0

Let me know if you'd like any more info. I think a lot of the confusion is that we are addressing multiple different concerns in this thread.

[SimonBaeumer]

You are right! I did store my scripts inside in a sub-directory, makes sense!
Commented on #131

[dylanhitt]

I think this can be closed now

[dylanhitt]

Closing, the shorting comings of --dir will be addressed in #131 and the shortcomings of calling additional assets in #136.