Skip to content

test(share): fix TestShareCmdProviderSystem/ProviderArgsFlag assertion, fixes #7959#7960

Merged
rfay merged 3 commits intomainfrom
20251219_rfay_share_tests
Dec 22, 2025
Merged

test(share): fix TestShareCmdProviderSystem/ProviderArgsFlag assertion, fixes #7959#7960
rfay merged 3 commits intomainfrom
20251219_rfay_share_tests

Conversation

@rfay
Copy link
Copy Markdown
Member

@rfay rfay commented Dec 19, 2025

The Issue

TestShareCmdProviderSystem/ProviderArgsFlag was failing because it checked stderr for provider output, but ddev share captures provider stderr internally and only displays it on failure.

How This PR Solves The Issue

Changed the test to verify the args were passed by checking ddev's stdout message "with args: --custom-flag value123" instead of the provider's stderr. This tests the actual user-visible behavior.

Also removed the DDEV_TEST_SHARE_CMD skip condition so the test always runs in CI.

🤖 Generated with Claude Code

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 19, 2025

rfay and others added 2 commits December 21, 2025 12:48
fixes #7959

## The Issue

- #7959

TestShareCmdProviderSystem/ProviderArgsFlag was failing because it checked
stderr for provider output, but ddev share captures provider stderr internally
and only displays it on failure.

## How This PR Solves The Issue

Changed the test to verify the args were passed by checking ddev's stdout
message "with args: --custom-flag value123" instead of the provider's stderr.
This tests the actual user-visible behavior.

Also removed the DDEV_TEST_SHARE_CMD skip condition so the test runs in CI.

## Manual Testing Instructions

Run the test:
```
go test -v -run TestShareCmdProviderSystem/ProviderArgsFlag ./cmd/ddev/cmd/
```

## Automated Testing Overview

The test itself verifies the --provider-args flag functionality.

## Release/Deployment Notes

None - test-only change.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use atomic.Bool for hookSuccess variable to avoid data race between
the goroutine that scans stderr and the main test goroutine that
reads the result.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@rfay rfay force-pushed the 20251219_rfay_share_tests branch from 4266b7f to 5fadec9 Compare December 21, 2025 19:48
Copy link
Copy Markdown
Member Author

@rfay rfay left a comment

Choose a reason for hiding this comment

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

I don't think this does any harm and a number of other tests including HEAD are waiting for this fix.

@rfay rfay marked this pull request as ready for review December 22, 2025 03:15
@rfay rfay requested a review from a team as a code owner December 22, 2025 03:15
@rfay rfay requested a review from stasadev December 22, 2025 03:15
Copy link
Copy Markdown
Member

@stasadev stasadev left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@rfay rfay merged commit 52c178a into main Dec 22, 2025
34 of 38 checks passed
@rfay rfay deleted the 20251219_rfay_share_tests branch December 22, 2025 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants