Conversation
| printStartUpMessage() | ||
|
|
||
| // TODO: proper subcommands | ||
| if flag.Arg(0) == "upload" { |
There was a problem hiding this comment.
Good for now. IF we have more than 1 subcommands, we might start thinking a better way to manage them.
There was a problem hiding this comment.
I have some slightly improved subcommand handling in another upcoming branch
| filename := flag.Arg(1) | ||
| if filename == "" { | ||
| return fmt.Errorf("expected filename for JUnit XML or JSON file") | ||
| } |
There was a problem hiding this comment.
I assume this is a file path, not necessarily a file name? Should we update the error message to be clear?
There was a problem hiding this comment.
Yeah true, there's always a bit of ambiguity re. whether “filename” means “just the basename” or “the full path”. I should clarify.
There was a problem hiding this comment.
I've updated the error message to say “path” not “filename”.
internal/upload/upload.go
Outdated
| var logArgs []interface{} | ||
| for k, v := range respData { | ||
| logArgs = append(logArgs, k, v) | ||
| } |
There was a problem hiding this comment.
[nit] What do you think of moving this to the Upload function?
There was a problem hiding this comment.
I think the logging belongs in the CLI layer; something else might want to do an Upload and get the response and log it in some other way (e.g. combined with some other information), or not at all.
I also think logging all the key/values in the response is too much (currently it's uuid and url and the latter contains the former)… if I change this to just log the upload URL from the response, then it'll be a one-line log call.
internal/upload/upload.go
Outdated
| func Upload(cfg Config, runEnv runEnvMap, format string, filename string) (map[string]string, error) { | ||
| body, err := buildUploadData(runEnv, format, filename) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("Upload: %w", err) |
There was a problem hiding this comment.
We print upload: %v in main function already, so I think this is redundant. Maybe
| return nil, fmt.Errorf("Upload: %w", err) | |
| return nil, fmt.Errorf("building upload data: %w", err) |
There was a problem hiding this comment.
Fixed!
Before:
2025/03/20 12:00:46 INFO Uploading key=0195b0a3-49bf-7812-af95-ff7ef0e6ff4d format=junit filename=test.xml
Buildkite Test Engine Client: upload: Upload: opening test.xml for reading: open test.xml: permission denied
After:
2025/03/20 12:00:46 INFO Uploading key=0195b0a3-49bf-7812-af95-ff7ef0e6ff4d format=junit filename=test.xml
Buildkite Test Engine Client: upload: preparing upload data: opening test.xml for reading: open test.xml: permission denied
2 participants
Teach
bktecto upload test results to Test Engine.