Skip to content

Conversation

@pradeep90
Copy link
Contributor

I have made things!

Thanks for the thoughtful and convenient plugin for running interface laws.

I found, however, that the plugin was not running the laws for an interface I defined myself. So, I decided to add that behavior.

Main changes:

  • We run laws from ancestor classes outside the returns module. This is the user-visible change.
  • In the hypothesis plugin, for each property test, we patch the interface with the container strategy:
    • Earlier, we were patching all ancestor classes, regardless of whether they had any laws. (We can see that in a test file before the fix)
    • Now, we are being more precise and patching only classes that have laws. But there should be no user-visible change. It's just cleaner.

Checklist

  • I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
  • I have created at least one test case for the changes I have made
  • I have updated the documentation for the changes I have made
  • I have added my changes to the CHANGELOG.md

Related issues

When running property tests for an container, we register it as the
strategy for each ancestor interface with laws.

However, we do that only for modules in `returns.`, which means that
lawful interfaces written by users will not be resolved. This commit
just demonstrates the current state.
Right now, it just gets all MRO classes, and it misses interfaces
defined outside `returns`. Will tackle that in the next commit.
Earlier, we were restricting it to any class within `returns`.

Main user-visible change: We now include `Lawful` ancestors outside `returns`.
We check that the interface class actually has laws. This should
eliminate any false positives that come from patching interfaces that
were defined outside `returns`.

Added a `_ParentWrapper` to test that it doesn't show up. (It did before
the change in this commit.)

Add to CHANGELOG and the hypothesis plugins page.
Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! Just one minor suggestion.

@codecov
Copy link

codecov bot commented Mar 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (82ef3ef) to head (7adb881).
Report is 344 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##            master     #2060    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           80        81     +1     
  Lines         2485      2576    +91     
  Branches       437        44   -393     
==========================================
+ Hits          2485      2576    +91     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks!

@sobolevn sobolevn merged commit 475dc52 into dry-python:master Mar 9, 2025
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants