MarvinJWendt / testza

Full-featured test framework for Go! Assertions, fuzzing, input testing, output capturing, and much more! 🍕
MIT License
418 stars 21 forks source link

Directories created with `testza.SnapshotCreateOrValidate` do not have execute permissions set #107

Closed jamestelfer closed 2 years ago

jamestelfer commented 2 years ago

When using the following:

err = testza.SnapshotCreateOrValidate(t, t.Name(), string(result))
testza.AssertNoError(t, err)

The first run succeeds and creates a snapshot. The second test run fails with the error:

"stat /[path redacted]/testdata/snapshots/TestReports/test_name.testza: permission denied"

Running ls -l on the parent directory shows drw------- for the testdata directory.

AFAICT, there are two paths for creating directories in snapshot.go: via SnapshotCreate, which ends up calling os.MkdirAll(dir, 0755).

https://github.com/MarvinJWendt/testza/blob/77ba3a4a1fd8f7de13711106f004362c54ad0846/snapshot.go#L30-L34

The second, via SnapshotCreateOrValidate, calls os.MkdirAll(path.Dir(snapshotPath), 0600):

https://github.com/MarvinJWendt/testza/blob/77ba3a4a1fd8f7de13711106f004362c54ad0846/snapshot.go#L130-L135

If I understand this correctly, the issue is that the permissions bits on the second call should match the first.

jamestelfer commented 2 years ago

HTH. I'm enjoying the library: great output, and the snapshot functions are really useful. Thanks!

MarvinJWendt commented 2 years ago

Hi @jamestelfer, I cannot reproduce it with my machine. But you're totally right, that the permission should be changed to be equal to the others. I will, most likely, push a release today, that includes this fix. It would be kind if you could report if it works then!

And thanks for the compliment! 😄

jamestelfer commented 2 years ago

No worries and thanks! I will definitely check it out.

I can only reproduce it when the snapshots are first created and there's no existing directory. I'm not sure either whether it's impacted by my use of t.Run() and t.Name() with a data driven test, which increases the levels of directory nesting.

I looked at creating a test case and a PR but I wasn't sure how to try to get the snapshots to be sourced from a separate folder. I can't say that I looked deeply at it though (sorry).

This was occurring for me on a Mac, haven't tried any other systems.

jamestelfer commented 2 years ago

OK I have a minimal repro:

func TestSnapshots(t *testing.T) {
    cases := []struct {
        name string
    }{
        {
            name: "nested-name",
        },
    }

    for _, c := range cases {
        t.Run(c.name, func(t *testing.T) {
            // this should fail as the snapshot file can't be written to the directory that doesn't exist
            err := testza.SnapshotCreateOrValidate(t, t.Name(), "flamingos are pink")
            testza.AssertNoError(t, err)
        })
    }
}

Let me see if I can get a failing test case out of this.

jamestelfer commented 2 years ago

That should do the trick, but feel free to change the test in whatever way you see fit. I had a linter issue that I've come across at work a few times, so hopefully #109 will fix that too.

Thanks for the project!

MarvinJWendt commented 2 years ago

Hi @jamestelfer, thanks for the PR! I am currently looking into it, and it looks good.