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

assert: allow updating expected vars/consts inside functions #244

Merged
merged 2 commits into from
Sep 25, 2022

Conversation

motemen
Copy link
Contributor

@motemen motemen commented Sep 20, 2022

Hi, thank you for such useful library. I was reading source and found -update flag to update the test code itself with "expectedXXX"-named variables. This was what I just wanted. Cool!

Currently the source-update functionality only supports toplevel names. This PR changes this behavior to rewrite vars/consts which resides deeper scope.

@CLAassistant
Copy link

CLAassistant commented Sep 20, 2022

CLA assistant check
All committers have signed the CLA.

@motemen motemen marked this pull request as ready for review September 20, 2022 15:02
@dnephin
Copy link
Member

dnephin commented Sep 23, 2022

Thank you for the PR! I was hoping to add support for this. I'll get CI working again and have a look at the change in more detail in the next few days

Copy link
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

Not sure why the CI run failed, it's working for me now.

This looks great! I pushed one small commit to rename a function to reflect the new return value.

Thanks again for the PR!

@dnephin dnephin force-pushed the update-expected-inside-func branch 2 times, most recently from f6fe867 to b40de99 Compare September 25, 2022 19:29
@dnephin dnephin merged commit 36dd5d1 into gotestyourself:main Sep 25, 2022
@motemen
Copy link
Contributor Author

motemen commented Sep 27, 2022

Thank you for quick merge! Looking forward to a new release 😋

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