First pass at a simple integration test #1353
Conversation
| @@ -131,23 +131,44 @@ func (s Resources) ProcessFileUpload(response http.ResponseWriter, request *http | |||
| defer request.Body.Close() | |||
| } | |||
|
|
|||
| if !IsValidContentTypeForUpload(request.Header) { | |||
| contentType, err := ParseUploadContentType(request.Header) | |||
There was a problem hiding this comment.
I'd like us to go back to using a pipeline approach (if/else chaining) for this set of requirements, primarily to keep it in line with other endpoints. It does come with the upside of not requiring any explicit returns.
| @@ -172,13 +193,20 @@ func (s Resources) ListAcceptedFileUploadTypes(response http.ResponseWriter, req | |||
| api.WriteBasicResponse(request.Context(), ingestModel.AllowedFileUploadTypes, http.StatusOK, response) | |||
| } | |||
|
|
|||
| func IsValidContentTypeForUpload(header http.Header) bool { | |||
| func ParseUploadContentType(header http.Header) (string, error) { | |||
There was a problem hiding this comment.
This should probably be private, I'm not sure why the previous was public tbh, but we can fix that here.
| return "", fmt.Errorf("missing Content-Type header") | ||
| } | ||
|
|
||
| parsed, _, err := mime.ParseMediaType(rawValue) | ||
| if err != nil { | ||
| return "", fmt.Errorf("invalid Content-Type format: %w", err) | ||
| } | ||
|
|
||
| if !slices.Contains(ingestModel.AllowedFileUploadTypes, parsed) { | ||
| return "", fmt.Errorf("unsupported Content-Type: %s", parsed) | ||
| } | ||
|
|
||
| return parsed, nil |
There was a problem hiding this comment.
Returning this to a pipeline style is preferred right now. There are no current edge cases in this logic
| "github.com/specterops/bloodhound/src/model" | ||
| ) | ||
|
|
||
| func TestIntegration_SaveIngestFile_JSON(t *testing.T) { |
There was a problem hiding this comment.
Let's name this TestSaveIngestFile and use subtests to split by JSON/zip/etc
| location, fileType, err := SaveIngestFile("/tmp", "application/json", body) | ||
| if err != nil { | ||
| t.Fatalf("SaveIngestFile failed: %v", err) | ||
| } | ||
|
|
||
| if fileType != model.FileTypeJson { | ||
| t.Errorf("expected fileType %v, got %v", model.FileTypeJson, fileType) | ||
| } | ||
|
|
||
| info, statErr := os.Stat(location) | ||
| if statErr != nil { | ||
| t.Fatalf("output file not found: %v", statErr) | ||
| } | ||
|
|
||
| if info.Size() == 0 { | ||
| t.Error("file saved is empty") | ||
| } |
There was a problem hiding this comment.
We have access to testify for require and assert assertions, it makes the test code a bit more concise and easier to reason about. I'd recommend using require.NoError for your error conditions that should never fail because they'd cause the remaining assertions to autofail, and appropriate assertions like assert.Equal to check size. See https://pkg.go.dev/github.com/stretchr/testify/assert#pkg-functions for more docs on what assertions exist. The more specific the better, since it acts as documentation for what specifically you're asserting.
Description
Describe your changes in detail
Motivation and Context
This PR addresses: [GitHub issue or Jira ticket number]
Why is this change required? What problem does it solve?
How Has This Been Tested?
Please describe in detail how you tested your changes.
Include details of your testing environment, and the tests you ran to
see how your change affects other areas of the code, etc.
Screenshots (optional):
Types of changes
Checklist: