search

tophat / syrupy

:pancakes: The sweeter pytest snapshot plugin
https://tophat.github.io/syrupy
Apache License 2.0
501 stars 33 forks source link

feat: Support passing arbitrary arguments/context to custom extensions (Issue #700) #814

Open atharva-2001 opened 10 months ago

atharva-2001 commented 10 months ago

Description

Related to https://github.com/tophat/syrupy/issues/687 and https://github.com/tophat/syrupy/issues/700.

Allowing custom **kwargs to custom extension classes.

e.g. This PR refactors the X submodule, applying Y's algorithm to improve Z by K percent.

Related Issues

Checklist

Additional Comments

No additional comments.

atharva-2001 commented 10 months ago

@noahnu I'm changing the classes which make up the AbstractSyrupyExtension class and also it's subclasses. Is this the right way to do this?

noahnu commented 10 months ago

We have built-in linters in the project via pyinvoke. You can run invoke lint and invoke test locally.

noahnu commented 10 months ago

Changing the signatures of all these functions will break any existing extensions in the wild, unless they were already using kwargs. Even then, I spent a couple hours trying to pass the context (or extra_args as you called it) to all relevant functions and the PR grew a bit out of control... I think we need to move back to instance-based extensions (which was something we moved away from in syrupy v4).

I've put up https://github.com/tophat/syrupy/pull/816 to add "context", although context is only available in instance extension methods and there aren't many of those. Once that change is in, we'll have to start migrating from classmethods to instance methods. May need to do that in a syrupy v5 update since it'll be a breaking change to the API.

Going back to instance methods also plays nicely with some of the discussion in https://github.com/tophat/syrupy/pull/754