Skip to content

APP-1823 - Support dry run in application CLI#62

Merged
shayshim merged 1 commit intomainfrom
feature/APP-1823
Feb 16, 2026
Merged

APP-1823 - Support dry run in application CLI#62
shayshim merged 1 commit intomainfrom
feature/APP-1823

Conversation

@shayshim
Copy link
Copy Markdown
Collaborator

@shayshim shayshim commented Feb 8, 2026

  • The pull request is targeting the main branch.
  • The code has been validated to compile successfully by running go vet ./....
  • The code has been formatted properly using go fmt ./....
  • All static analysis checks passed.
  • All tests have passed. If this feature is not already covered by the tests, new tests have been added.
  • All E2E tests have passed.
  • All changes are detailed at the description.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Feb 8, 2026

Test Results

20 tests  +1   19 ✅ +2   2m 8s ⏱️ +39s
 2 suites ±0    1 💤 ±0 
 1 files   ±0    0 ❌  - 1 

Results for commit e6ef7e9. ± Comparison against base commit ae4e5d1.

♻️ This comment has been updated with latest results.

}

if response.StatusCode != http.StatusCreated {
// Dry run returns http.StatusOK
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

After Yuri's PR is merged, let's make sure it's valid also when async == true.

expectedError: "",
},
{
name: "success with dry-run (201 Created)",
Copy link
Copy Markdown
Collaborator

@asafgabai asafgabai Feb 9, 2026

Choose a reason for hiding this comment

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

Why do we need to test both 200 and 201? Does this request return 201 sometimes?

}

if response.StatusCode != expectedStatusCode {
if !apphttp.IsSuccessStatusCode(response.StatusCode) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm ok with this approach, but it'd be better to change it for all commands (or use the previous approach 🙃)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I changed other instances in the same file to use the new approach, which is a bit more readable

endpoint := fmt.Sprintf("/v1/applications/%s/versions/", request.ApplicationKey)
response, responseBody, err := ctx.GetHttpClient().Post(endpoint, request, map[string]string{"async": strconv.FormatBool(!sync)})
response, responseBody, err := ctx.GetHttpClient().Post(endpoint, request,
map[string]string{"async": strconv.FormatBool(!sync), "dry_run": strconv.FormatBool(dryRun)})
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Let's add E2E test

@shayshim shayshim merged commit 1ade6c2 into main Feb 16, 2026
8 of 10 checks passed
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.

2 participants