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.

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

feat: update ftltest for injected verb clients #2877

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

worstell
Copy link
Contributor

No description provided.

@worstell worstell requested review from alecthomas and a team as code owners September 27, 2024 18:21
@worstell worstell requested review from deniseli and removed request for a team September 27, 2024 18:21
This was referenced Sep 27, 2024
Copy link
Contributor

@deniseli deniseli left a comment

Choose a reason for hiding this comment

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

This is awesome!!

moduleCtx := modulecontext.FromContext(ctx).CurrentContext()
override, err := moduleCtx.BehaviorForVerb(schema.Ref{Module: ref.Module, Name: ref.Name})
if err != nil {
return resp, fmt.Errorf("%s: %w", ref, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we make this "test harness failed to retrieve behavior for verb %s: %w" and similar make L535 something like "test harness failed to call verb %s: %w"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good idea!

@alecthomas
Copy link
Collaborator

Don't forget you'll need to update the customer code to use this!

@worstell
Copy link
Contributor Author

Don't forget you'll need to update the customer code to use this!

have been working this up today :)

@worstell worstell force-pushed the worstell/20240926-inject-verbs-test-changes branch 2 times, most recently from 9c537e5 to b3fc9a1 Compare September 27, 2024 22:05
@worstell
Copy link
Contributor Author

think i'll wait til monday to merge this one because it will require customer changes, and i'd ideally like to couple those changes with the announcement/change for the new verb client stuff - which is much more invasive for a friday 😄

@worstell worstell force-pushed the worstell/20240926-inject-verbs-test-changes branch from 9feb877 to 090a931 Compare September 30, 2024 18:33
@worstell worstell merged commit 4ebc30b into main Sep 30, 2024
92 checks passed
@worstell worstell deleted the worstell/20240926-inject-verbs-test-changes branch September 30, 2024 18:43
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.

3 participants