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.

Sign up for GitHub

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

Jump to bottom

Flake: TestPlugins_CatchStderr #2213

Closed
Tracked by #2906
carolynvs opened this issue Jun 29, 2022 · 2 comments · Fixed by #2236
Closed
Tracked by #2906

Flake: TestPlugins_CatchStderr #2213

carolynvs opened this issue Jun 29, 2022 · 2 comments · Fixed by #2236
Assignees
@carolynvs
Labels
tech debt 💸 Make it easier to develop Porter
Projects
Porter and Mixins

Comments

@carolynvs
Copy link
Member

I've found that this test is flaking out sometimes. The cause appears to be when the plugin fails but doesn't print to stderr. The PluginConnection code formats the error differently depending on if it captured anything to stderr or not.

porter/pkg/plugins/pluggable/connection.go

Lines 158 to 161 in 59c4a56

if err != nil {
if stderr := errbuf.String(); stderr != "" {
err = fmt.Errorf("could not connect to the %s plugin: %w: %s", c.key, err, stderr)
} 

2022-06-29T15:21:33.5652721Z === RUN   TestPlugins_CatchStderr
2022-06-29T15:21:33.5653045Z === RUN   TestPlugins_CatchStderr/plugin_throws_an_error
2022-06-29T15:21:33.5653413Z     pluggable_integration_test.go:59: 
2022-06-29T15:21:33.5653859Z         	Error Trace:	pluggable_integration_test.go:59
2022-06-29T15:21:33.5654210Z         	Error:      	Error message not equal:
2022-06-29T15:21:33.5655272Z         	            	expected: "could not connect to the secrets.testplugin.vault plugin: Unrecognized remote plugin message: \n\nThis usually means that the plugin is either invalid or simply\nneeds to be recompiled to support the latest protocol.: i am just test plugin. i don't function\n"
2022-06-29T15:21:33.5656209Z         	            	actual  : "Unrecognized remote plugin message: \n\nThis usually means that the plugin is either invalid or simply\nneeds to be recompiled to support the latest protocol."
2022-06-29T15:21:33.5656791Z         	Test:       	TestPlugins_CatchStderr/plugin_throws_an_error
2022-06-29T15:21:33.5657399Z --- FAIL: TestPlugins_CatchStderr (0.76s)
2022-06-29T15:21:33.5657946Z     --- FAIL: TestPlugins_CatchStderr/plugin_throws_an_error (0.03s)

I'm not sure why sometimes that test plugin outputs to stderr and sometimes it doesn't? I may just be confused though. 😅
@carolynvs carolynvs added the tech debt 💸 Make it easier to develop Porter label Jun 29, 2022
@carolynvs carolynvs mentioned this issue Jul 8, 2022
Merged
4 tasks
@carolynvs carolynvs self-assigned this Jul 8, 2022
@carolynvs
Copy link
Member Author

I think that there may be a race condition in go-plugins where we aren't guaranteed to have the plugin's error message written to the error buffer that we passed to the framework before it reads the bad message written by the plugin to stdout.

carolynvs added a commit to carolynvs/porter that referenced this issue Jul 8, 2022
@carolynvs
Fix test flake TestPlugins_CatchStderr
396994c 
The cause of the inconsistent error message returned when connecting to
a plugin was that there is a bit of a race condition in the
hashicorp/go-plugin framework where messages written to the plugins
stderr may not be reported by the time the plugin framework rejects a
non-conformant message that the plugin wrote to stdout.

The result is that sometimes our code would get a chance to see the
erorr message that the plugin wrote, and sometimes we wouldn't.

I don't want to modify the plugin framework just make one of our tests
more reliable. So instead I have updated our test plugin so that it
waits a bit between writing to stderr and writing to stdout. Now we
reliably get both messages captured by go-plugins.

In addition, I made the error message returned by the plugin connection
consistent regardless of whether or not we were able to capture stderr
from the plugin.

Fixes getporter#2213

Signed-off-by: Carolyn Van Slyck <me@carolynvanslyck.com>
Porter and Mixins automation moved this from Inbox to Done Jul 11, 2022
@github-actions
Copy link

Closed by #2236.

@schristoff schristoff mentioned this issue Sep 13, 2023
Open
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@carolynvs carolynvs

Labels
tech debt 💸 Make it easier to develop Porter
Projects
Porter and Mixins
  
Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix test flake TestPlugins_CatchStderr carolynvs/porter
1 participant
@carolynvs