Skip to content

add test for successful jws tests#36

Merged
dpetran merged 2 commits intomainfrom
fix/policy-test
Feb 2, 2024
Merged

add test for successful jws tests#36
dpetran merged 2 commits intomainfrom
fix/policy-test

Conversation

@dpetran
Copy link
Contributor

@dpetran dpetran commented Jan 31, 2024

The request body needs to be json encoded if it's sent with json headers, just a string will cause weird/broken behavior.

The request body needs to be json encoded if it's sent with json headers, just a string
will cause weird/broken behavior.
@dpetran dpetran requested a review from a team January 31, 2024 22:24
(-> query-res :body json/read-value first (get "f:assert")))
"policy opts prevented seeing bob's secret"))))
(testing "JWS requests"
(let [txn-req {"@context" ["https://ns.flur.ee" test-system/default-context]
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add more commentary to these tests with more testing macro calls?

What is this setup code before the test assertion doing? Why should this transaction be authorized? Why should the following one not be?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole deftest block sets up a simple graph which allows users to view and update their own ex:secret predicates. The jws block of tests verifies that using the identity can be correctly verified and used for policy enforcement via the jws mechanism of signing requests.

Before, there were only two cases: that Alice couldn't modify Bob's secret and that Bob's secrets don't appear in Alice's query results. I added the test case for Alice being able to modify her own secret.

Copy link
Contributor

@zonotope zonotope left a comment

Choose a reason for hiding this comment

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

I think the test documentation here could be better, but that would require overhauling the whole file, which is beyond the scope of this pr.

I do think we need to do a better job of our test documentation in general however. I think the testing and is macros should be set up to provide a full English sentence describing the whole setup of each test and what should happen on each assertion. This will make it easier for others who did not write the code or tests to understand what the intentions are.

I think his is something we should keep in mind moving forward, but this specific pr is 👍🏾

@dpetran dpetran merged commit 5f5ead9 into main Feb 2, 2024
@dpetran dpetran deleted the fix/policy-test branch February 2, 2024 17:02
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.

2 participants