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

Test Generation Auditor #4444

Merged
merged 14 commits into from
Aug 25, 2023
Merged

Test Generation Auditor #4444

merged 14 commits into from
Aug 25, 2023

Conversation

Dargones
Copy link
Collaborator

@Dargones Dargones commented Aug 19, 2023

This PR (number 4444:) introduces a test generation auditor that scans the program for anything that will cause test generation to fail (errors) or might lead to less helpful results (warnings). These are reported via ConsoleErrorReporter like Dafny would report any other problems. Dafny returns a non-zero exit code and terminates execution if any errors are present or if there are warnings and the --warn-as-errors flag is on. Here is the list of all errors/warnings that can be emitted.

Errors:

  • Cannot find a method or function annotated with {:testEntry} attribute
  • Program is not wrapped in a module. Put your code inside "module M {}" or equivalent
  • {:testInline} attribute on the method [METHOD] can only take one argument, which must be a positive integer specifying the recursion unrolling limit (absence of such an argument or 1 means no unrolling)
  • Test Generation does not support abstract types, array types, and trait types as inputs. Consider disallowing values of type [TYPE] to be part of the input to {:testEntry}-annotated method [METHOD]
  • Any error from parser or resolver is propagated up and reported as well

Warnings:

  • Method [METHOD] will not be inlined because it is not reachable from any of the {:testEntry}-annotated methods or functions
  • Test Generation does not fully support function types as inputs. Consider disallowing values of type [TYPE] to be part of the input to {:testEntry}-annotated method [METHOD]
  • Cannot find witness for type [TYPE]. Please consider adding a witness to the declaration
  • Method [METHOD] is annotated with {:timeLimit 300} but test generation is called with --verification-time-limit:20. Consider increasing the time limit for test generation.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@Dargones
Copy link
Collaborator Author

As a side note, I noticed that several tests throughout the project use the Utils.Parse method from DafnyTestGeneration to parse the input. Is there perhaps a better place I should move it to?

@Dargones
Copy link
Collaborator Author

Dargones commented Aug 23, 2023

Update: I brought the errors reporting mechanism in line with how it works in Dafny more generally - warnings are colored in yellow, errors in red, and -warn-as-errors flag treats warnings like errors

Copy link
Member

@atomb atomb left a comment

Choose a reason for hiding this comment

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

The implementation looks great, and I mostly had comments about terminology, both in messages and internal names. The key one is that I'm not sure that distinguishing this from the rest of the test generation code with the name "auditor" has a lot of purpose. It's just the first step of test generation, in my mind.

@@ -14,7 +14,7 @@ public enum ErrorLevel {
}

public enum MessageSource {
Parser, Cloner, RefinementTransformer, Rewriter, Resolver, Translator, Verifier, Compiler, Documentation
Parser, Cloner, RefinementTransformer, Rewriter, Resolver, Translator, Verifier, Compiler, Documentation, TestGenerationAuditor
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that the Auditor term adds much here. I'd just call this TestGeneration. And the checks performed here are just the first phase of test generation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This makes sense. Have made the change


namespace DafnyTestGeneration.Test;

public class AuditorTests : Setup {
Copy link
Member

Choose a reason for hiding this comment

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

I might call this WarningTests or something, to avoid confusion with the separate feature that uses the name "auditor".

Copy link
Member

Choose a reason for hiding this comment

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

It's great to have lots of tests of this, BTW. :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done!

/// </summary>
private void PrintWarningsAndErrors(ErrorReporter errorReporter, Program program) {
if (Errors.Count() != 0) {
errorReporter.Message(MessageSource.TestGenerationAuditor, ErrorLevel.Error, "", program.StartToken,
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you can't use the more concise Error method instead of the Message method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, I think I just wasn't aware that can use Error/Message - have changed the code to use them now

}
}
if (Warnings.Count() != 0) {
errorReporter.Message(MessageSource.TestGenerationAuditor, ErrorLevel.Warning, "", program.StartToken,
Copy link
Member

Choose a reason for hiding this comment

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

Similarly to errors, can you use Warning here?

}
if (type is UserDefinedType userDefinedType) {
var genericMessage =
$"Consider disallowing values of type {userDefinedType} to be part of the input to " +
Copy link
Member

Choose a reason for hiding this comment

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

The word "disallowing" seems weird here. Maybe use terminology similar to what you use elsewhere about writing a wrapper to generate types that can't be constructed automatically out of types that can?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have paraphrased this to "Consider modeling values of type TYPE with a datatype and passing them as input to the {:testEntry} annotated method". I agree that something along these lines should be better than "disallowing" since it tells the user what to do to prevent the error.

@Dargones
Copy link
Collaborator Author

Thank you, @atomb! I have made all the changes and have also renamed Auditor to FirstPass to avoid confusion with the top-level Auditor functionality.

Copy link
Member

@atomb atomb left a comment

Choose a reason for hiding this comment

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

This looks great!

@atomb atomb enabled auto-merge (squash) August 25, 2023 15:10
@atomb atomb merged commit 95eb74b into dafny-lang:master Aug 25, 2023
18 checks passed
keyboardDrummer pushed a commit to keyboardDrummer/dafny that referenced this pull request Sep 15, 2023
This PR (number 4444:) introduces a test generation auditor that scans
the program for anything that will cause test generation to fail
(errors) or might lead to less helpful results (warnings). These are
reported via `ConsoleErrorReporter` like Dafny would report any other
problems. Dafny returns a non-zero exit code and terminates execution if
any errors are present or if there are warnings and the
`--warn-as-errors` flag is on. Here is the list of all errors/warnings
that can be emitted.

Errors:
- Cannot find a method or function annotated with {:testEntry} attribute
- Program is not wrapped in a module. Put your code inside \"module M
{}\" or equivalent
- {:testInline} attribute on the method [METHOD] can only take one
argument, which must be a positive integer specifying the recursion
unrolling limit (absence of such an argument or 1 means no unrolling)
- Test Generation does not support abstract types, array types, and
trait types as inputs. Consider disallowing values of type [TYPE] to be
part of the input to {:testEntry}-annotated method [METHOD]
- Any error from parser or resolver is propagated up and reported as
well

Warnings:
- Method [METHOD] will not be inlined because it is not reachable from
any of the {:testEntry}-annotated methods or functions
- Test Generation does not fully support function types as inputs.
Consider disallowing values of type [TYPE] to be part of the input to
{:testEntry}-annotated method [METHOD]
- Cannot find witness for type [TYPE]. Please consider adding a witness
to the declaration
- Method [METHOD] is annotated with {:timeLimit 300} but test generation
is called with --verification-time-limit:20. Consider increasing the
time limit for test generation.

By submitting this pull request, I confirm that my contribution
is made under the terms of the MIT license.

---------

Co-authored-by: Aleksandr Fedchin <fedchina@amazon.com>
keyboardDrummer pushed a commit that referenced this pull request Sep 19, 2023
This PR (number 4444:) introduces a test generation auditor that scans
the program for anything that will cause test generation to fail
(errors) or might lead to less helpful results (warnings). These are
reported via `ConsoleErrorReporter` like Dafny would report any other
problems. Dafny returns a non-zero exit code and terminates execution if
any errors are present or if there are warnings and the
`--warn-as-errors` flag is on. Here is the list of all errors/warnings
that can be emitted.

Errors:
- Cannot find a method or function annotated with {:testEntry} attribute
- Program is not wrapped in a module. Put your code inside \"module M
{}\" or equivalent
- {:testInline} attribute on the method [METHOD] can only take one
argument, which must be a positive integer specifying the recursion
unrolling limit (absence of such an argument or 1 means no unrolling)
- Test Generation does not support abstract types, array types, and
trait types as inputs. Consider disallowing values of type [TYPE] to be
part of the input to {:testEntry}-annotated method [METHOD]
- Any error from parser or resolver is propagated up and reported as
well

Warnings:
- Method [METHOD] will not be inlined because it is not reachable from
any of the {:testEntry}-annotated methods or functions
- Test Generation does not fully support function types as inputs.
Consider disallowing values of type [TYPE] to be part of the input to
{:testEntry}-annotated method [METHOD]
- Cannot find witness for type [TYPE]. Please consider adding a witness
to the declaration
- Method [METHOD] is annotated with {:timeLimit 300} but test generation
is called with --verification-time-limit:20. Consider increasing the
time limit for test generation.

By submitting this pull request, I confirm that my contribution
is made under the terms of the MIT license.

---------

Co-authored-by: Aleksandr Fedchin <fedchina@amazon.com>
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.

None yet

2 participants