Skip to content

Add file assertion #147

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

Merged
merged 7 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

- Add ability to test suite from a url
- Add ability to test suite from stdin
- Add `file` assertion to `stdout` and `stderr`

# v2.3.0

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ tests:
object.attr: hello # Make assertions on json objects
xml:
"//book//auhtor": Steven King # Make assertions on xml documents
file: correct-output.txt
exit-code: 127
skip: false

Expand Down
2 changes: 1 addition & 1 deletion commander_unix.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ tests:
contains:
- ✓ [local] it should exit with error code
- "- [local] it should skip, was skipped"
line-count: 17
line-count: 19
exit-code: 0

it should assert that commander will fail:
Expand Down
10 changes: 10 additions & 0 deletions integration/unix/commander_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ tests:
/books/0/author: J. R. R. Tokien
/books/1/author: Joanne K. Rowling

it should assert file contents on stdout:
command: cat ./integration/unix/_fixtures/big_out.txt
stdout:
file: ./integration/unix/_fixtures/big_out.txt

it should assert file contents on stderr:
command: cat ./integration/unix/_fixtures/big_out.txt >&2
stderr:
file: ./integration/unix/_fixtures/big_out.txt

it should inherit from parent env:
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind adding a failing case as well?

config:
inherit-env: true
Expand Down
33 changes: 33 additions & 0 deletions pkg/matcher/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"github.com/antchfx/xmlquery"
"github.com/pmezard/go-difflib/difflib"
"github.com/tidwall/gjson"
"io/ioutil"
"log"
"strings"
)
Expand All @@ -20,6 +21,7 @@ const (
NotContains = "notcontains"
JSON = "json"
XML = "xml"
File = "file"
)

// NewMatcher creates a new matcher by type
Expand All @@ -37,6 +39,8 @@ func NewMatcher(matcher string) Matcher {
return JSONMatcher{}
case XML:
return XMLMatcher{}
case File:
return FileMatcher{}
default:
panic(fmt.Sprintf("Validator '%s' does not exist!", matcher))
}
Expand Down Expand Up @@ -240,3 +244,32 @@ to be equal to

return result
}

// FileMatcher matches output captured from stdout or stderr
// against the contents of a file
type FileMatcher struct {
}

func (m FileMatcher) Match(got interface{}, expected interface{}) MatcherResult {
expectedText, err := ioutil.ReadFile(expected.(string))
if err != nil {
panic(err.Error())
}
expectedString := string(expectedText)

result := got == expectedString

diff := difflib.UnifiedDiff{
A: difflib.SplitLines(got.(string)),
B: difflib.SplitLines(expectedString),
FromFile: "Got",
ToFile: "Expected",
Context: 3,
}
diffText, _ := difflib.GetUnifiedDiffString(diff)

return MatcherResult{
Diff: diffText,
Success: result,
}
}
24 changes: 20 additions & 4 deletions pkg/matcher/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,16 @@ func TestTextMatcher_ValidateFails(t *testing.T) {

func TestEqualMatcher_Validate(t *testing.T) {
m := EqualMatcher{}
got := m.Match(1, 1)
got := m.Match(2, 2)
assert.True(t, got.Success)
}

func TestEqualMatcher_ValidateFails(t *testing.T) {
m := EqualMatcher{}
got := m.Match(1, 0)
got := m.Match(2, 3)
assert.False(t, got.Success)
assert.Contains(t, got.Diff, "+0")
assert.Contains(t, got.Diff, "-1")
assert.Contains(t, got.Diff, "+3")
assert.Contains(t, got.Diff, "-2")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that there was always a "-1" in the diff output because it tells the user how many lines changed, so this makes the test a tiny bit less brittle.

}

func TestContainsMatcher_Match(t *testing.T) {
Expand Down Expand Up @@ -183,3 +183,19 @@ another`
assert.False(t, r.Success)
assert.Equal(t, diff, r.Diff)
}

func TestFileMatcher_Validate(t *testing.T) {
m := FileMatcher{}
got := m.Match("zoom", "zoom.txt")
Copy link
Member

@dylanhitt dylanhitt Oct 20, 2020

Choose a reason for hiding this comment

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

I'm okay with reading directly from the filesystem here, there's not really a good way to mock ioutil.ReadFile, that I know of. The best thing I've found is this, and even this would require some code change. I personally think this is too much for well tested code already.

Another direction that could be taken is removing the actual code needing to be tested and placing it in a private helper function. For example, it wouldn't be stretch to write a helper function that could be used in both FileMatcher and TextMatcher, and just perform the I/O in the FileMatcher and then call the helper method. I'm neither here nor there on this approach.

Copy link
Member

@dylanhitt dylanhitt Oct 20, 2020

Choose a reason for hiding this comment

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

One thing @SimonBaeumer if we take the approach from reading from the FS should we designate a folder to do this?

Or a temp file could be created and then used/removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about the temp file approach but I wanted input since that didn't feel great either. Ordinarily I'd probably accept a Reader instead of a filename but then we're back to the original question of where does the file actually get opened.

Copy link
Member

Choose a reason for hiding this comment

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

Right, the only other thing I can really think of is to FileMatcher take an argument for a function, that basically wraps ioutil.ReadFile(filePath string). Like:

type TestingMocking struct {
	readFile func(filePath string) ([]byte, error)
}

You could then setup FileMatcher in NewMatcher.
And then:

func (m FileMatcher) Match(got interface{}, expected interface{}) MatcherResult {
	buf, err := m.readFile(expected.(string))
        // rest goes here 

This would allow us to mock readFile. Again I think this may be too robust. I think I'll defer @SimonBaeumer on this as I don't really have preference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I'll try that out and see how it ends up.

Copy link
Member

@SimonBaeumer SimonBaeumer Oct 23, 2020

Choose a reason for hiding this comment

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

Hey Together,
so yes, ideally we would mock the filesystem but this not often done and not necessary imho.
I am totally fine with using libraries and access to filesystem or env variables.

The downside of mocking is in general that it gets very complex and often takes a lot of effort to maintain.
Mocking is nice if there is some complex things I want to replace, like network connections, tasks that need a lot of time or simply complex stuff to setup, i.e. you would need to run a kubernetes cluster.

Go ignores directories starting with underscores, like _assets and directories matching testdata.
Just put the asset inside a pkg/matcher/testdata directory.
You should be able access it like this:

wd, _ := os.Getwd()
asset := path.Join(wd, "testdata/file.txt")
content, _ := ioutil.ReadFile(asset)

The testing framework of go will execute all tests with the working directory pointing to the currently executed file, so it is easy to access the testdata.

Mocking:

Another option like @dylanhitt mentioned is to use dependency injection. That means that we inject (normally as input parameters in the constructor or functions) dependencies on other structs (or classes).
These classes / structs do implement an interface, therefore you could replace the implementation inside your tests with a fake one.

Original implementation, i.e. in file_matcher.go:

type FileReaderInterface interface  {
    func Read(file string) string
}
type FileReader struct {}
func (r *FileReader) Read (file strinng) string {
    ioutil.ReadFile...
    // direct access to filesystem
}

type FileMatcher struct {
    Reader FileReaderInterface
}
func NewFileMatcher() *FileMatcher {
    var reader FileReaderInterface
    reader = FileReader{}
    return &FileMatcher{
        Reader: reader,
    }
}

In your test file_matcher_test.go you would than do something like this:

type FileReaderMock {}
func Read(file string) string {
    if file == "test.txt" {
        return "Mocked this for a test"
    }

    if file == "test2.txt" {
       return "mocked for another test"
    }
    // maybe panic if the file wasn't mocked
    panic("file was not mocked")
}

func TestFileMatcher(t *testing.T) {
    var readerMock FileReaderInterface
    readerMock = FileReaderMock{}

    // Inject the reader mock into the FileMatcher
    matcher := FileMatcher{Reader: readerMock}
} 

Nevertheless, I am totally happy with both approaches.

assert.True(t, got.Success)
assert.Equal(t, "", got.Diff)
}

func TestFileMatcher_ValidateFails(t *testing.T) {
m := FileMatcher{}
got := m.Match("zoom", "zoologist.txt")
assert.False(t, got.Success)
println(got.Diff)
assert.Contains(t, got.Diff, "+zoologist")
assert.Contains(t, got.Diff, "-zoom")
}
1 change: 1 addition & 0 deletions pkg/matcher/zoologist.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
zoologist
1 change: 1 addition & 0 deletions pkg/matcher/zoom.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
zoom
1 change: 1 addition & 0 deletions pkg/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ type ExpectedOut struct {
NotContains []string `yaml:"not-contains,omitempty"`
JSON map[string]string `yaml:"json,omitempty"`
XML map[string]string `yaml:"xml,omitempty"`
File string `yaml:"file,omitempty"`
}

// CommandUnderTest represents the command under test
Expand Down
7 changes: 7 additions & 0 deletions pkg/runtime/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,13 @@ func validateExpectedOut(got string, expected ExpectedOut) matcher.MatcherResult
}
}

if expected.File != "" {
m = matcher.NewMatcher(matcher.File)
if result = m.Match(got, expected.File); !result.Success {
return result
}
}

return result
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/suite/yaml_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ func (y *YAMLSuiteConf) convertToExpectedOut(value interface{}) runtime.Expected
"lines",
"json",
"xml",
"file",
"not-contains":
break
default:
Expand All @@ -242,6 +243,11 @@ func (y *YAMLSuiteConf) convertToExpectedOut(value interface{}) runtime.Expected
exp.Exactly = toString(exactly)
}

// Parse file key
if file := v["file"]; file != nil {
exp.File = toString(file)
}

//Parse line-count key
if lc := v["line-count"]; lc != nil {
exp.LineCount = lc.(int)
Expand Down