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

Export FakeFrame.Arg2KVIterator to faciliate tests #757

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

cinchurge
Copy link
Collaborator

With arg2KVIterator unexported, it is extremely difficult for consumers to test with call frames when arg2 is involved. This change simply exports that attr so that is is possible to supply a custom iterator, such as via arg2.NewKeyValIterator.

@codecov
Copy link

codecov bot commented Aug 23, 2019

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2818e4c) 88.21% compared to head (878ca2f) 88.36%.
Report is 149 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #757      +/-   ##
==========================================
+ Coverage   88.21%   88.36%   +0.14%     
==========================================
  Files          40       40              
  Lines        4074     4074              
==========================================
+ Hits         3594     3600       +6     
+ Misses        364      361       -3     
+ Partials      116      113       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -100,7 +100,7 @@ type FakeCallFrame struct {
Arg2StartOffsetVal, Arg2EndOffsetVal int
IsArg2Fragmented bool

arg2KVIterator arg2.KeyValIterator
Arg2KVIterator arg2.KeyValIterator
hasArg2KVIterator error
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to export hasArg2KVIterator as well.

@prashantv
Copy link
Contributor

For someone creating a fake call frame, does it make more sense to provide a setter that takes a map[string]string that it uses to create the iterator?

Similarly, should we merge Arg2StartOffset and Arg2EndOffset into a single Arg2Size? I think that's the only use-case we have for the values right?

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