massdriver-cloud / massdriver-cli

Deprecated. See https://github.com/massdriver-cloud/mass
https://massdriver.cloud
Apache License 2.0
3 stars 2 forks source link

Bugfix/copy local #108

Closed WillBeebe closed 2 years ago

WillBeebe commented 2 years ago

Issue:

When I moved from my recursive copy method to using filepath.Walk I introduced a bug. filepath.Walk behaves differently when copying from local root vs non-local root.

// tests
copyFrom: testdata/bundle
relative path: /

// real-world bundle build 
copyFrom: .
relative path: .

Because this behaves differently for local, the tests didn't catch it.

This wasn't fun: Write code -> test with bundle -> write test -> rewrite code -> tests pass -> declare victory -> accept defeat.

So I tried this: Write test that captures the bug -> fix code -> test with bundle

Here's a run with the new test and without the changes (test fails).

git stash
Saved working directory and index state WIP on copy-local: 66eafa9 add test for local

make test
go test ./cmd
?       github.com/massdriver-cloud/massdriver-cli/cmd  [no test files]
go test ./pkg/...
ok      github.com/massdriver-cloud/massdriver-cli/pkg/application  (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/bundle   0.221s
?       github.com/massdriver-cloud/massdriver-cli/pkg/cache    [no test files]
ok      github.com/massdriver-cloud/massdriver-cli/pkg/client   (cached)
Comparing (want) testdata/copy-from-root/utils.go and (got) utils.go
=865
--- FAIL: TestCopyFolder (0.00s)
    --- FAIL: TestCopyFolder/CopyFromRoot (0.00s)
        copy_test.go:86: got h1:47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU=, want h1:uvsIsiJ8JZ/oX+4EsTYQJHsLIZsW2r4NqSiT0pl1mjA=
FAIL
FAIL    github.com/massdriver-cloud/massdriver-cli/pkg/common   0.122s
ok      github.com/massdriver-cloud/massdriver-cli/pkg/compress (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/definition   (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/jsonschema   (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/provisioners/terraform   0.135s
ok      github.com/massdriver-cloud/massdriver-cli/pkg/template (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/version  (cached)
FAIL
make: *** [test] Error 1

New test, with the changes

git stash pop

make test
go test ./cmd
?       github.com/massdriver-cloud/massdriver-cli/cmd  [no test files]
go test ./pkg/...
ok      github.com/massdriver-cloud/massdriver-cli/pkg/application  (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/bundle   0.420s
?       github.com/massdriver-cloud/massdriver-cli/pkg/cache    [no test files]
ok      github.com/massdriver-cloud/massdriver-cli/pkg/client   (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/common   0.338s
ok      github.com/massdriver-cloud/massdriver-cli/pkg/compress (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/definition   (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/jsonschema   (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/provisioners/terraform   0.350s
ok      github.com/massdriver-cloud/massdriver-cli/pkg/template (cached)
ok      github.com/massdriver-cloud/massdriver-cli/pkg/version  (cached)
coryodaniel commented 2 years ago

What would be nice on publish would be to use the allowlist of files (yaml, schemas, step dirs) and validate that those are in the bundle and fail if they are not.

I wouldn't add it to this branch unless that's a super quick change as I'd like to get this merged pronto. FF to add a ticket for the above if its not a fast one.

WillBeebe commented 2 years ago

@jaketf and @coryodaniel this is ready to go. The test is now sane and I ran through the CLI locally publishing a bundle to my private org.