-
Notifications
You must be signed in to change notification settings - Fork 17
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
Write Tests for the ToString methods in auth_logic/ast.h and datalog/program.h #720
Comments
Consider also extending https://github.com/google-research/raksha/blob/main/src/ir/auth_logic/ast_string.h so that it can be used instead of these methods. |
This was referenced Sep 20, 2022
Closed
copybara-service bot
pushed a commit
that referenced
this issue
Sep 22, 2022
These were originally made as part of #654, but are split into this separate PR in order to keep PRs more singular in purpose. Tests of ToString will still be added later in a separate PR in order to satisfy #720. Closes #727 COPYBARA_INTEGRATE_REVIEW=#727 from google-research:tostring-improve@aferr adb85e9 PiperOrigin-RevId: 476146199
copybara-service bot
pushed a commit
that referenced
this issue
Oct 3, 2022
This adds tests to `ast_string.h` which we apparently already had and already used for other tests in the repository. However, this code didn't have any tests itself. So rather than have two ways of making a string from the AST, I updated `ast_string.h` so that it is good enough for the uses of the ToString methods in the AST and added some tests for `ast_string.h`. In a follow-up PR, I will deprecate the ToString methods in the AST class and use just `ast_string.h` for the remaining tests thereby satisfying #720. Closes #731 COPYBARA_INTEGRATE_REVIEW=#731 from google-research:to-string-tests@aferr bbf1048 PiperOrigin-RevId: 478516570
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
(Note that these ToString methods were previously called DebugPrint)
It turns out that DebugPrint was actually used to implement
auth_logic_ast_traversing_visitor_test.cc
and between me and Gogul we were not able to think of a way to re-implement this test without using strings within a reasonable timeframe (see also: this code).Instead of deprecating this method, we should write tests for
ToString
. Then it's OK for tests to depend on it.The text was updated successfully, but these errors were encountered: