Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stable API to get test methods and datasets from data providers for metaprogramming #6015

Open
donquixote opened this issue Oct 27, 2024 · 7 comments
Labels
type/enhancement A new idea that should be implemented

Comments

@donquixote
Copy link

Use case

I am working on tests tool based on phpunit that work with generated data files per test method and dataset:

  • In "recording" mode, data is written to the data files.
  • In "default" mode, data is compared with the data files, and a mismatch causes a test failure.

The data files are tracked in version control.

Part of this tool should also be to identify left-over generated data files.
To do this, it would need to get a list of test methods and available datasets.

So far, with PhpUnit 9.x, I did all this using a trait RecordedTestTrait, with a test method testLeftoverRecordedFiles() that would find any leftover files, using PHPUnit\Util\Test::getProvidedData() and comparing that to the directory contents.

Problem

Currently, in PhpUnit 10.x and 11.x, classes and methods I found that retrieve a list of datasets from data providers are marked as @internal, and they changed between 9.x and 10.x:

  • PHPUnit\Metadata\Api\DataProvider exists since 10.x, and is marked as internal.
  • PHPUnit\Framework\TestBuilder exists since 10.x (afaik), and is marked as internal.
  • PHPUnit\Util\Test::getProvidedData() existed in 9.x, and was marked as internal.

In the past I used the "internal" methods from PhpUnit 9.x, being aware of the risk.
Now I am upgrading to 10.x (as a consequence of other package updates), and I need to rethink this part.

Request

Provide public and stable methods to retrieve methods and datasets for a test class.
It seems the existing methods were already stable during the major versions..

Workaround

For now I will probably solve this with a github action that will wipe and re-generate all recordings, and then fail on git diff.
This will at least identify leftover files in the CI pipeline.
It is not convenient for local development, though.

@donquixote donquixote added the type/enhancement A new idea that should be implemented label Oct 27, 2024
@sebastianbergmann
Copy link
Owner

Are you sure you need an API (in the form of methods that you call) to retrieve the data you need for the "recording" mode you mention? Getting "a list of test methods and available datasets" should be achievable by subscribing to the appropriate events.

@donquixote
Copy link
Author

Are you sure you need an API (in the form of methods that you call) to retrieve the data you need for the "recording" mode you mention? Getting "a list of test methods and available datasets" should be achievable by subscribing to the appropriate events.

I am not sure which events you imagine for this task, but..

We could use events that fire on every call to a test method, and collect the list "somewhere".
If the tests run with any filter, instead of running the complete test class, then that collected list of calls and datasets will be incomplete. And I don't know if we can reliably detect that a filter was used.

I also see the DataProviderMethodCalled, but I don't see that it would provide the data list.
I searched for "data" and "Data" in the src/Event/Events directory in PhpUnit 10.5, and did not find any events that deliver the data list, but maybe I missed something.

If we want to fail in a dedicated test method like testNoLeftoverRecordings(), we need to make the data that was obtained in whichever event listener available to that test method.
We cannot rely on object properties for this, we would have to write a tmp file somewhere.
I would also want this to work if a user just runs with --filter=testNoLeftoverRecordings.

To the workaround mentioned above:
If this check happens in the CI pipeline, or somehow outside of phpunit itself, then we don't need to worry about which filters were used etc.

@donquixote
Copy link
Author

Also, when writing to the recording file, I need to construct the file name from the test name and the dataset name.
In PhpUnit 9.x this was $this->getName(), in PhpUnit 10+ it is $this->name().
But both are marked as internal.
Perhaps this part can be solved with events.

@sebastianbergmann
Copy link
Owner

Please describe exactly what information you need.

@donquixote
Copy link
Author

To give some context, this is the trait as it exists now, which works with PhpUnit 9.x.
https://github.com/ock-php/ock-mono/blob/main/packages/testing/src/RecordedTestTrait.php

And this is an example for a test recording with dynamically discovered route definitions in a Drupal module.
https://github.com/ock-php/ock-mono/blob/main/modules/ock/tests/recordings/Kernel/OckRoutingTest-testRoutes.yml

For a regular test method it works like this:

  • Before a test method runs, or at the first time ->assertAsRecorded() is called:
    • In regular mode: The data from a recording file is loaded. The file name is based on the method name and dataset name. The data is basically a list of "records".
  • A test method may call ->assertAsRecorded($data, $label) any number of times. When this happens:
    • In regular mode: The first/next record is picked from the recorded data, and compared to [$label => $data]. If they are different, the test fails.
    • In recording mode: The [$label => $data] is added to an array property in the test class.
  • After the test method is complete:
    • In regular mode: We check if the recorded data has any leftover records. If so, the test fails.
    • In recording mode: If the test did not fail for other reasons, we write the recorded data to the recording file.

Then, the testLeftoverRecordedFiles() compares the provided test names to the stored file names. If there is a mismatch, it fails.

Some notes:

  • A test method can call assertAsRecorded() any number of times, but it can also call other assertion methods, for expectations that are not meant to be updated. This means even in recording mode, a test can still fail.
  • Typically, the data sent to assertAsRecorded() will be normalized and cleaned up, to avoid or reduce failures caused by meaningless noise.

So, what is the information I need?

  • Before a test starts, or during a test in assertAsRecorded(), I need to know the test name and dataset name, to determine which yml file to load. In recording mode, it is enough to have this information after the test method has finished.
  • In a previous version, I wanted to output a snapshot of the dataset data at the top of a recording. I changed my mind about this, too much noise to have it in the recorded yml file. You will see the leftover call to $this->getProvidedData() in buildYamlHeader(), which I should simply remove.
  • In testLeftoverRecordedFiles(), I would need the full list of methods and dataset names for that test class.

@donquixote
Copy link
Author

So, what is the information I need?

I need to know the test name and dataset name

I think I could use an event listener to get these names, and store them in a property in the class.
But it is more convenient to just call $this->getName() or $this->name() and $this->dataName().

@donquixote
Copy link
Author

So, what is the information I need?

Actually, I notice now that in tearDownRecorder() method (with @after annotation) I need to know if the test was successful or not. With PhpUnit 9.x I would call $this->hasFailed(). It seems in PhpUnit 10.x I can use $this->status(). But both are/were marked as internal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A new idea that should be implemented
Projects
None yet
Development

No branches or pull requests

3 participants
@sebastianbergmann @donquixote and others