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

Added none operation #1297

Merged

Conversation

AlgoStephenAkiki
Copy link
Contributor

Resolves #1283

Adds none operation to fields

Summary

Explain the goal of this change and what problem it is solving. Format this cleanly so that it may be used for a commit message, as your changes will be squash-merged.

Test Plan

How did you test these changes? Please provide the exact scenarios you tested in as much detail as possible including commands, output and rationale.

Resolves #1283

Adds none operation to fields
@codecov
Copy link

codecov bot commented Oct 31, 2022

Codecov Report

Merging #1297 (e50b046) into conduit (ea8c774) will increase coverage by 0.03%.
The diff coverage is 82.35%.

@@             Coverage Diff             @@
##           conduit    #1297      +/-   ##
===========================================
+ Coverage    58.82%   58.86%   +0.03%     
===========================================
  Files           78       78              
  Lines        10490    10506      +16     
===========================================
+ Hits          6171     6184      +13     
- Misses        3786     3788       +2     
- Partials       533      534       +1     
Impacted Files Coverage Δ
processors/filterprocessor/fields/filter.go 80.70% <82.35%> (+0.21%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines +43 to +45
if err != nil {
return data.BlockData{}, err
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like the only bit that isn't covered by the unit tests. What are the failure conditions which would cause the searcher to return an error? Is this conditional something that should never happen or it could happen if you provide an invalid pattern?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It just allows the searcher interface to propagate errors if the implementation needs to. A quick example is the microalgos searcher which can return an error if the field is not a microalgo type (sort of belt and suspenders, but I think it is useful).

output, err := fp.Process(bd)
assert.NoError(t, err)
assert.Equal(t, len(output.Payset), 1)
assert.Equal(t, output.Payset[0].SignedTxnWithAD.SignedTxn.Txn.PaymentTxnFields.Receiver, sampleAddr2)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe assign the txn to a var and then assert equal on output.Payset[0] and that txn.

@AlgoStephenAkiki AlgoStephenAkiki merged commit 5043497 into conduit Nov 2, 2022
@AlgoStephenAkiki AlgoStephenAkiki deleted the 1283-conduit-filter-plugin-none-operation branch November 2, 2022 15:56
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.

4 participants