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

Improve CSV writer tests #7851

Merged
merged 8 commits into from
Apr 21, 2021
Merged

Conversation

vuule
Copy link
Contributor

@vuule vuule commented Apr 5, 2021

Closes #6836

Overall reduces test execution time and improves coverage:

  • Replace around 2.5K test cases with multiple tests that only vary related options.
  • Correctly verify the output with multicharacter line_terminator option (not supported by readers).
  • Add a seed call before the random generator is used in one of the tests.
  • Simplify a few tests by removing irrelevant comparisons.
  • Use buffer output instead of file in affected tests (could be applied to many more).

@vuule vuule added tests Unit testing for project Python Affects Python cuDF API. cuIO cuIO issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Apr 5, 2021
@vuule vuule self-assigned this Apr 5, 2021
@vuule vuule marked this pull request as ready for review April 5, 2021 18:11
@vuule vuule requested a review from a team as a code owner April 5, 2021 18:11
@codecov
Copy link

codecov bot commented Apr 6, 2021

Codecov Report

Merging #7851 (4d95cd2) into branch-0.20 (51336df) will decrease coverage by 0.36%.
The diff coverage is 88.34%.

❗ Current head 4d95cd2 differs from pull request most recent head cbfc64d. Consider uploading reports for the commit cbfc64d to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##           branch-0.20    #7851      +/-   ##
===============================================
- Coverage        82.88%   82.52%   -0.37%     
===============================================
  Files              103      103              
  Lines            17668    17300     -368     
===============================================
- Hits             14645    14277     -368     
  Misses            3023     3023              
Impacted Files Coverage Δ
python/cudf/cudf/core/column/__init__.py 100.00% <ø> (ø)
python/cudf/cudf/io/orc.py 86.80% <ø> (-0.10%) ⬇️
python/cudf/cudf/utils/cudautils.py 55.04% <25.00%> (-2.72%) ⬇️
python/cudf/cudf/core/column/numerical.py 94.41% <70.00%> (-0.02%) ⬇️
python/cudf/cudf/core/column/column.py 88.48% <71.42%> (-0.17%) ⬇️
python/dask_cudf/dask_cudf/backends.py 89.20% <80.00%> (-0.38%) ⬇️
python/cudf/cudf/core/dataframe.py 90.75% <83.33%> (-0.11%) ⬇️
python/cudf/cudf/utils/utils.py 89.47% <91.66%> (-0.04%) ⬇️
python/cudf/cudf/core/column/datetime.py 89.40% <100.00%> (-0.52%) ⬇️
python/cudf/cudf/core/column/timedelta.py 88.33% <100.00%> (-0.34%) ⬇️
... and 58 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4893259...cbfc64d. Read the comment docs.

@vuule
Copy link
Contributor Author

vuule commented Apr 7, 2021

rerun tests

@vuule vuule requested a review from brandon-b-miller April 20, 2021 19:25
@vuule vuule added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 4 - Needs cuDF (Python) Reviewer labels Apr 20, 2021
@vuule
Copy link
Contributor Author

vuule commented Apr 21, 2021

rerun tests

@kkraus14
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 5b71fca into rapidsai:branch-0.20 Apr 21, 2021
@vuule vuule deleted the bug-csv-writer-tests branch November 11, 2021 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge cuIO cuIO issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change Python Affects Python cuDF API. tests Unit testing for project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Many CSV writer tests are redundant
4 participants