Skip to content

MINOR: Add initial property tests for StandardAuthorizer#12703

Merged
hachikuji merged 4 commits into
apache:trunkfrom
hachikuji:minor-start-property-tests
Oct 4, 2022
Merged

MINOR: Add initial property tests for StandardAuthorizer#12703
hachikuji merged 4 commits into
apache:trunkfrom
hachikuji:minor-start-property-tests

Conversation

@hachikuji

Copy link
Copy Markdown
Contributor

In #12695, we discovered a gap in our testing of StandardAuthorizer. We addressed the specific case that was failing, but I think we need to establish a better methodology for testing which incorporates randomized inputs. This patch is a start in that direction. We implement a few basic property tests using jqwik which focus on prefix searching. It catches the case from #12695 prior to the fix. In the future, we can extend this to cover additional operation types, principal matching, etc.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@hachikuji hachikuji force-pushed the minor-start-property-tests branch from c2b85db to 4c8b32a Compare September 29, 2022 23:14

@mumrah mumrah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@hachikuji this looks really nice, I like the approach.

Do you know if jqwik supports seeding the Random? It might be nice to be able to see which seed was used for a run and later be able to supply it for a local run when reproducing issues. E.g., if a random test case trips an exception, we won't get to printCounterExample

@hachikuji

Copy link
Copy Markdown
Contributor Author

@mumrah Failures in jqwik always display the inputs including the random seed. I added the logic in printCounterExample mainly so that I didn't need to do the mapping of the inputs to the corresponding ACLs in my head.

@hachikuji

Copy link
Copy Markdown
Contributor Author

Here is an example of what jqwik displays after a failure:

                              |-------------------jqwik-------------------
tries = 1                     | # of calls to property
checks = 1                    | # of not rejected calls
generation = RANDOMIZED       | parameters are randomly generated
after-failure = PREVIOUS_SEED | use the previous seed
when-fixed-seed = ALLOW       | fixing the random seed is allowed
edge-cases#mode = MIXIN       | edge cases are mixed in
edge-cases#total = 0          | # of all combined edge cases
edge-cases#tried = 0          | # of edge cases tried in current run
seed = 4172772880591196336    | random seed to reproduce generated values

Shrunk Sample (10 steps)
------------------------
  arg0: java.util.Random@33fdefbd
  arg1: "A"
  arg2: []

Original Sample
---------------
  arg0: java.util.Random@33fdefbd
  arg1: "kAtwnJNLw5qXQ"
  arg2:
    [
      "Z-3N", 
      "ItgCxbTB6ggnl.UoO3GSABofVi9rQ1lpZvEADaiFFvU9sn0JHgDCf6or1XpcBA8RifuOO6tny-EkDhca3Hbou3g6XjxrdMJuO2pChT5YNQElhO7Zl.Wzabb5nnxVvnyJLLQJZH8YGA9imEjtiYTICcS3W5NZQHxQ4RNOd7odx.ItDeu5hPHh0tw6EgVSVpmX-QI9yaexUNiJeD3liFS7LP7sVGH91kDGgU0sktk.g8evo3IH83kCP3z.YYqCJDL"
    ]

  Original Error
  --------------
  org.opentest4j.AssertionFailedError:
    expected: <ALLOWED> but was: <DENIED>

@mumrah mumrah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @hachikuji, the output looks neat. We should start doing more of this type of testing :)

@hachikuji hachikuji merged commit c5745d2 into apache:trunk Oct 4, 2022
cmccabe pushed a commit that referenced this pull request Oct 24, 2022
In #12695, we discovered a gap in our testing of `StandardAuthorizer`. We addressed the specific case that was failing, but I think we need to establish a better methodology for testing which incorporates randomized inputs. This patch is a start in that direction. We implement a few basic property tests using jqwik which focus on prefix searching. It catches the case from #12695 prior to the fix. In the future, we can extend this to cover additional operation types, principal matching, etc.

Reviewers: David Arthur <mumrah@gmail.com>
guozhangwang pushed a commit to guozhangwang/kafka that referenced this pull request Jan 25, 2023
In apache#12695, we discovered a gap in our testing of `StandardAuthorizer`. We addressed the specific case that was failing, but I think we need to establish a better methodology for testing which incorporates randomized inputs. This patch is a start in that direction. We implement a few basic property tests using jqwik which focus on prefix searching. It catches the case from apache#12695 prior to the fix. In the future, we can extend this to cover additional operation types, principal matching, etc.

Reviewers: David Arthur <mumrah@gmail.com>
rutvijmehta-harness pushed a commit to rutvijmehta-harness/kafka that referenced this pull request Feb 9, 2024
In apache#12695, we discovered a gap in our testing of `StandardAuthorizer`. We addressed the specific case that was failing, but I think we need to establish a better methodology for testing which incorporates randomized inputs. This patch is a start in that direction. We implement a few basic property tests using jqwik which focus on prefix searching. It catches the case from apache#12695 prior to the fix. In the future, we can extend this to cover additional operation types, principal matching, etc.

Reviewers: David Arthur <mumrah@gmail.com>
rutvijmehta-harness added a commit to rutvijmehta-harness/kafka that referenced this pull request Feb 9, 2024
… (#34)

In apache#12695, we discovered a gap in our testing of `StandardAuthorizer`. We addressed the specific case that was failing, but I think we need to establish a better methodology for testing which incorporates randomized inputs. This patch is a start in that direction. We implement a few basic property tests using jqwik which focus on prefix searching. It catches the case from apache#12695 prior to the fix. In the future, we can extend this to cover additional operation types, principal matching, etc.

Reviewers: David Arthur <mumrah@gmail.com>

Co-authored-by: Jason Gustafson <jason@confluent.io>
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.

3 participants