Skip to content
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

TraverserTests #2732

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

TraverserTests #2732

wants to merge 4 commits into from

Conversation

mawhite-msft
Copy link

Added tests with mocked responses for zc_traverser_blob

a.Nil(err)

//the mock server responds with a status code error
srv.AppendResponse(mock_server.WithStatusCode(403), mock_server.WithHeader("x-ms-error-code", "BlobNotFound"), mock_server.WithBody([]byte(MockErrorBody("BlobNotFound"))))
Copy link
Member

Choose a reason for hiding this comment

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

nit: BlobNotFound is usually a 404 in http world. If we want to use 403, we should take a look at common error codes for Blob and choose an appropriate one.

Copy link
Member

@adreed-msft adreed-msft left a comment

Choose a reason for hiding this comment

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

Looking good, nice work!

return nil
}, []ObjectFilter{})
a.Equal(map[string]StoredObject{}, seenFiles)
a.Equal("this blob uses customer provided encryption keys (CPK). At the moment, AzCopy does not support CPK-encrypted blobs. If you wish to make use of this blob, we recommend using one of the Azure Storage SDKs", err.Error())
Copy link
Member

Choose a reason for hiding this comment

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

This might be out of scope, but for these more specific errors, perhaps we should just have a zt_traverser_errors.go file where we store all of these strings in one place, that way, if it gets used in multiple places, or changed in the future, it's a zero-brainpower operation to find all the places to change it in tests and such.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants