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: call constant methods occasionally and allow assertion testing them #363

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

0xalpharush
Copy link
Contributor

closes #168

@0xalpharush
Copy link
Contributor Author

0xalpharush commented Jun 25, 2024

This will also close #55 but I'm not sure about it's last requirement

Ensure the corpus.UpdateCorpusAndCoverageMaps method checks if the last call was to a view method. If it was, do not record the call sequence in the corpus at that step, as it's not a coverage-increasing sequence we'd be interested in recording, as it was not state changing.

This can be left for later work, if desired, but I don't followed why we'd special case the last call. cc @Xenomega

Copy link
Collaborator

@anishnaik anishnaik left a comment

Choose a reason for hiding this comment

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

Overall, it looks really good. The only concern is that I don't think your test case is being called at the moment.

Additionally, I'm thinking if we move the TestViewMethods config variable outside of the AssertionTesting structure and put it in Testing. I'm thinking that testing view methods as a concept is more general than assertion testing or property testing. So maybe, we just push it up one level in the hierarchy. what do you think?

fuzzing/fuzzer_test.go Show resolved Hide resolved
@0xalpharush
Copy link
Contributor Author

0xalpharush commented Jul 2, 2024

Additionally, I'm thinking if we move the TestViewMethods config variable outside of the AssertionTesting structure and put it in Testing. I'm thinking that testing view methods as a concept is more general than assertion testing or property testing. So maybe, we just push it up one level in the hierarchy. what do you think?

I understand this config to mean that assertion failures in constant methods will be treated as failure if assertion mode is enabled, but I see your point that this will affect the sequences generated for property mode as well. I suppose we could have CallViewMethods up one level and it enables assertion testing constant methods (if true) but also allows completely filtering out all view/pure methods (if false) in either mode.

We could capture this in an issue and decide this after merging and implementing filtering ( I want to replace #275 to be compatible with these changes and fix several bugs in its implementation)

@anishnaik
Copy link
Collaborator

Additionally, I'm thinking if we move the TestViewMethods config variable outside of the AssertionTesting structure and put it in Testing. I'm thinking that testing view methods as a concept is more general than assertion testing or property testing. So maybe, we just push it up one level in the hierarchy. what do you think?

I understand this config to mean that assertion failures in constant methods will be treated as failure if assertion mode is enabled, but I see your point that this will affect the sequences generated for property mode as well. I suppose we could have CallViewMethods up one level and it enables assertion testing constant methods (if true) but also allows completely filtering out all view/pure methods (if false) in either mode.

We could capture this in an issue and decide this after merging and implementing filtering ( I want to replace #275 to be compatible with these changes and fix several bugs in its implementation)

Yeah I think this is fine for now and we can figure this out later

@0xalpharush 0xalpharush merged commit 98d3724 into master Jul 2, 2024
12 checks passed
@0xalpharush 0xalpharush deleted the feat/constant-method-testing branch July 2, 2024 18:33
s4nsec pushed a commit that referenced this pull request Jul 9, 2024
…them (#363)

* feat: call constant methods ocassionally and allow assertion testing them

* add test
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.

Assertion checking in view/pure functions is never reached
2 participants