I want to encourage beginners to contribute by making refactoring pull requests

[WarrenTheRabbit]

I have started to learn the codebase and I would like to perform refactorings. In part, I am hoping to make it easier for others to get involved. This issue was created to discuss the idea that this could be an improvement.

What's the problem this improvement will solve?

As someone that is not particularly clever, I get confused by long functions with multiple responsibilities. I have found the complexity and length of some of the code blocks in the repository a barrier to entry. I think this makes it harder for people to engage with the codebase. Why is that a problem?

Testing is important. It is also hard and uninspiring to some. But cars will crash and medical equipment will deliver lethal doses when inadequately tested. So testing is needed - especially as the length of code's shadow grows. On LinkedIn, a lot of veterans comment that their programming mindset and methodology were largely determined by what they enjoyed and/or were exposed to at the start of their programming journey. This makes me think it would be helpful if programmers had formative experiences in a passionate and beginner-friendly testing community.

If I can make the pytest codebase more accessible to beginners like me, maybe programmers will find some of the passion and professionalism they will need to make the world safer by being part of the pytest community. I would also work at supporting new arrivals, doing outreach and creating documentation as I become more seasoned. But that is in the future and ultimately separate from the proposal.

Describe the solution you'd like

As I learn the codebase, I would like to perform opportunistic and simple Extract Function/Method refactors. I'd consider doing other refactorings as my skills increase but my proposal for now is to keep things simple. The algorithm would be

1. Do Extract Function/Method
2. Type annotate it
3. Write its docstring
4. Commit the refactoring: refactor(optional scope): summary and a body for additional information and questions.

An example of my proposed refactoring approach can be seen by applying the algorithm to the argument-parsing responsibilities of _prepareconfig(...):

The argument-parsing behaviour of _prepareconfig delegated to the function get_parsed_args.

#src/_pytest/config/__init__.py
def _prepareconfig(
    args: Optional[Union[List[str], "os.PathLike[str]"]] = None,
    plugins: Optional[Sequence[Union[str, _PluggyPlugin]]] = None,
) -> "Config":
    args = get_parsed_args(args) <--- THIS
    config = get_config(args, plugins)
    . . .
)

get_parsed_args with annotations and docstring.

#src/_pytest/config/__init__.py
def get_parsed_args(
    args: Optional[Union[List[str], "os.PathLike[str]"]] = None
) -> List[str]:
    """Return `args` as a list of strings.

    This function ensures that process command line arguments (:data:`sys.argv`)
    are read in and that the rest of the application can treat command line
    arguments as a list of strings.

    :param args:
        List of command line arguments.
        If `args` is `None` or not given, defaults to reading arguments
        directly from the process command line (:data:`sys.argv`).
        If an :class:`os.PathLike` object, it is converted to a path string.

    :returns: A list of strings.

    :raises TypeError:
        If `args` is not `None`, not an :class:`os.PathLike` object, and not
        a list.

    """
    if args is None:
        args = sys.argv[1:]
    elif isinstance(args, os.PathLike):
        args = [os.fspath(args)]
    elif not isinstance(args, list):
        msg = (  # type:ignore[unreachable]
            "`args` parameter expected to be a list of strings, got: {!r} (type: {})"
        )
        raise TypeError(msg.format(args, type(args)))

    return args

The commit

refactor(config): extract argument-parsing

Responsibility for parsing arguments is removed from the body of _prepareconfig and encapsulated in its own function, get_parsed_args.

Questions

1. The documentation of os.PathLike says it can return a bytes object.
   get_parsed_args is type annotated to return a list of strings.
   Is that a problem?

2. Are f-strings to be avoided for backward compatibility reasons?

3. Does the use of a hyperlink in the commit body couple this commit to a source file in an undesirable way?

Alternative Solutions

To restate, I hope to make it easier for people to get involved. I want people to have the opportunity to find intellectual, social and emotional welcome in the pytest community. That would be a satisfying end in itself. But I also hope to increase the number of people who have a passion for something I think is very important:

• testing systems and making them safer; and
• building and maintaining testing tools so that others can make systems safer.

I think outreach and documentation of all stripes could achieve this. For example, one documentation idea is Case Study documents. Case Study documents would give an example of how a beginner (me) learned how to contribute to pytest. An example of that could be: documenting the work I do on changing the --fixtures-per-test output to ignore the parameterize mark's pseudo-fixtures. Of course, documentation ideas are not true alternatives - they can support rather than compete with the proposal.

Additional context

A worry I have is that my enthusiasm to contribute creates more work for the maintainers than it is worth to them on balance. My dream is not necessarily their dream and I feel self-conscious on this point. I do not want to take up their hours when it is my brain and heart - not theirs - motivating me to make this proposal. I've never liked the idea of getting others to do my work for me.
To that end, what steps would I need to take and what skills would I need to develop and demonstrate in order to be trusted with authority so that I can take the extra load this proposal might create - especially if it caught the interest of others - off the current maintainer's shoulders?

[RonnyPfannschmidt]

Thanks for this write up,

As I currently don't have the time for a complete answer, I'll start with a short one and will elaborate a more complete later

A basic gist is, that a lot of pytest grew very organic over the years.

This has loads of extra complexities that both core devs and some newcomers like you are starting to chip at.

A key detail for those Changes are scope and impact.

For example there's a tremendous amount of excellent and hard work coming in for fixtures state and dependencies that will help to eventually land some details we wanted to Land since 2016

This also first came in as one large Block we could not safely manage and now that contributor is chunking them up into pieces we can discuss and Progress individually.

Also some internals are of tremendous complexity due to very diligent backwards compatibility

For example it took me more than 5 attempts to reshape the Initial markers onto Something more manageable, this happened over the span of a few years.

The key to all of this is Vision and manageable steps that are communicated and anticipated

I believe your Vision is an important part of the picture and when we are handling the communication about timelines and steps well it's something where excellent progress can be done.

[WarrenTheRabbit]

@RonnyPfannschmidt: Thanks for this write up . . . I currently don't have the time for a complete answer . . . and will elaborate [later].

Take your time, Ronny. 💛 There is no rush. Your sand timer is as full as you have need. ⌛ When you do elaborate, I will respond with a summary of what has been discussed as a check on our understandings.

Meanwhile, as I have time, I will look through issues and documentation to increase my understanding of where you've been and where you are going. I am very naive (about planning, performance requirements, backward compatibility, labour supply, etc) so I want to actively address those naiveties.

[seanjedi]

I agree with this sentiment as someone who is starting to contribute to this repository, that some functions seem a bit overloaded and could be split up a bit to make the functionality a bit more clear.
IE cacheprovider seems to have some functions that could be two functions instead of one large one,
Like this one: https://github.com/pytest-dev/pytest/blob/main/src/_pytest/cacheprovider.py#L336