Skip to content

credentials/alts: fix defer in TestDial #7301

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
Merged
merged 4 commits into from
Jun 13, 2024
Merged

credentials/alts: fix defer in TestDial #7301

merged 4 commits into from
Jun 13, 2024

Conversation

aranjans
Copy link
Contributor

@aranjans aranjans commented Jun 5, 2024 •
edited
Loading

fix: second call in the defer isn't executing, which prevents the hsDialer restore and hence it was never getting updated. This PR fixes the TestDial.

RELEASE NOTES: n/a

@aranjans aranjans requested review from purnesh42H and arvindbr8 June 5, 2024 05:54
@aranjans aranjans added this to the 1.65 Release milestone Jun 5, 2024
purnesh42H
purnesh42H approved these changes Jun 5, 2024
View reviewed changes
Copy link
Contributor

@purnesh42H purnesh42H left a comment

Choose a reason for hiding this comment

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

lgtm for fix

@aranjans
Copy link
Contributor Author

aranjans commented Jun 5, 2024

Special thanks to @arp242 for identifying the bug here

@arvindbr8 arvindbr8 changed the title fix: defer in Test/Dial credentials/alts: fix defer in TestDial Jun 5, 2024
arvindbr8
arvindbr8 suggested changes Jun 5, 2024
View reviewed changes
Copy link
Member

@arvindbr8 arvindbr8 left a comment

Choose a reason for hiding this comment

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

one question tho. Also while you are here, do you mind capturing the intend of the test as a docstring to TestDial?

@aranjans aranjans assigned arvindbr8 and unassigned aranjans Jun 6, 2024
@aranjans aranjans force-pushed the aranjans_fix_def branch from dab452f to 5256bff Compare June 6, 2024 14:43
@arvindbr8 arvindbr8 modified the milestones: 1.65 Release, 1.66 Release Jun 6, 2024
@arvindbr8 arvindbr8 assigned aranjans and unassigned arvindbr8 Jun 6, 2024
@arvindbr8
Copy link
Member

Please update the test comment to actually say what it does.

purnesh42H
purnesh42H reviewed Jun 7, 2024
View reviewed changes
easwars
easwars reviewed Jun 7, 2024
View reviewed changes
@aranjans aranjans force-pushed the aranjans_fix_def branch 2 times, most recently from 51a3b23 to d280ecb Compare June 12, 2024 10:10
purnesh42H
purnesh42H reviewed Jun 12, 2024
View reviewed changes
purnesh42H
purnesh42H reviewed Jun 12, 2024
View reviewed changes
@aranjans aranjans removed their assignment Jun 12, 2024
easwars
easwars requested changes Jun 12, 2024
View reviewed changes
@aranjans aranjans assigned easwars and unassigned aranjans Jun 13, 2024
arvindbr8
arvindbr8 approved these changes Jun 13, 2024
View reviewed changes
Copy link
Member

@arvindbr8 arvindbr8 left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of this!

@arvindbr8 arvindbr8 merged commit 24a6b48 into grpc:master Jun 13, 2024
11 checks passed
@tbg tbg mentioned this pull request Nov 27, 2024
Closed
16 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@purnesh42H purnesh42H purnesh42H approved these changes

@easwars easwars easwars approved these changes

@arvindbr8 arvindbr8 arvindbr8 approved these changes

Assignees

@easwars easwars

Labels
Type: Bug
Projects
None yet
Milestone
1.66 Release
Development

Successfully merging this pull request may close these issues.
4 participants
@aranjans @arvindbr8 @easwars @purnesh42H