[SPARK-58480][PYTHON][TEST] Declare DataFrame golden test cases as methods of a test class - #57691
[SPARK-58480][PYTHON][TEST] Declare DataFrame golden test cases as methods of a test class#57691dtenedor wants to merge 2 commits into
Conversation
|
Thank you @dtenedor! cc @zhengruifeng |
gaogaotiantian
left a comment
There was a problem hiding this comment.
Generally, I believe this is the correct direction. We should have a relatively complicated mixin and many easy to read/reproduce test cases. I'm glad that we can get rid of a lot of files.
I have some comments about the implementation. It's a framework and I hope the code is cleaner and easier for extension in the future.
I also have some suggestions for next steps, but they are not for this PR.
| __file_metadata__ | ||
| --! source | ||
| df_golden/group_by | ||
| pyspark.sql.tests.df_golden.test_group_by.GroupByGoldenTests |
There was a problem hiding this comment.
I hope eventually we can get rid of our own file format and use something more generic like toml - which are also super readable by both humans and machines. We won't need to maintain our own parser for each section anymore. But this is not part of this PR.
There was a problem hiding this comment.
Yes, currently there are a lot of these .out golden files for testing in Apache Spark (example [1]). We could consider changing that, but it would be a bigger change than this PR would do in one go.
[1]
What changes were proposed in this pull request?
This PR follows up on #57122 and #57639 by replacing the per-case Python snippet files of the DataFrame golden test framework with a single test class per golden file.
The files under
python/pyspark/sql/tests/df_golden/scripts/group_by/are replaced by one module,python/pyspark/sql/tests/df_golden/test_group_by.py, holding one class whose_test_<case>methods each build and return the DataFrame under test:This is ordinary Python: imports are at the top of the module, cases can share helpers, and
sparkis a parameter rather than a name injected into anexec()namespace.Cases are ordinary unit tests:
DFGoldenTestMixin(a mixin, following the existingpyspark.testing.goldenutils.GoldenFileTestMixinpattern) registers a realtest_<case>method for every_test_<case>method, carrying its docstring over. Each case is reported individually and can be run on its own:Why are the changes needed?
The framework as merged put each case's DataFrame program in its own file, which drew several objections in the review of #57122:
exec()'d withsparkinjected into their namespace, which is why they needed anF821lint exemption and a RAT exclusion, and why they could not be imported or run outside the framework..testfile used to run as one unittest method, so a failure named a case but gave no way to run just that case. Now every case is a test method that can be selected by name, and the case body is an importable method.Does this PR introduce any user-facing change?
No. This is test-only: the framework and its test corpus.
How was this patch tested?
This PR is test-only. The migration was verified as follows.
pyspark.sql.tests.df_golden.test_df_golden_frameworkpasses (51 tests).Was this patch authored or co-authored using generative AI tooling?
Generated-by: Cursor (Opus 5)