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

test: improve structure of table tests #256

Merged
merged 112 commits into from
May 5, 2023

Conversation

robmeth
Copy link
Contributor

@robmeth robmeth commented Apr 28, 2023

Closes #98

Summary of Changes

Regarding the tests of Table:

Rename tests so they start with test_should_
Use fixtures and parametrization to reduce code duplication
Add ids to parametrized tests
Cover everything (see this) - goal: 100% coverage
Use the tests for Row and Column as an example.

Co-authored-by: daniaHu 129186516+daniaHu@users.noreply.github.com
Co-authored-by: jxnior01 129027012+jxnior01@users.noreply.github.com

robmeth and others added 20 commits April 28, 2023 13:14
…sts' into 98-improve-structure-of-table-tests
…sts' into 98-improve-structure-of-table-tests
…sts' into 98-improve-structure-of-table-tests
…sts' into 98-improve-structure-of-table-tests
@robmeth robmeth self-assigned this Apr 28, 2023
@robmeth robmeth requested review from a team and lars-reimann as code owners April 28, 2023 14:25
@robmeth robmeth linked an issue Apr 28, 2023 that may be closed by this pull request
@lars-reimann
Copy link
Member

lars-reimann commented Apr 28, 2023

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON black 45 0 0 1.05s
✅ PYTHON mypy 45 0 1.82s
✅ PYTHON ruff 45 0 0 0.05s
✅ REPOSITORY git_diff yes no 0.02s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #256 (edf937f) into main (bb5a07e) will increase coverage by 100.00%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           main      #256        +/-   ##
===========================================
+ Coverage      0   100.00%   +100.00%     
===========================================
  Files         0        44        +44     
  Lines         0      1705      +1705     
===========================================
+ Hits          0      1705      +1705     

see 44 files with indirect coverage changes

@lars-reimann
Copy link
Member

Suggestion for the tests that check whether some error is raised: Instead of putting the type of the error in the name of the test, put the condition under which an error should be raised there.

Example (for remove_column): Call it test_should_raise_if_column_does_not_exist instead of test_should_raise_UnknownColumnNameError.

Copy link
Member

@lars-reimann lars-reimann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very nice at first glance. The part

Cover everything (see this) - goal: 100% coverage

seems to be missing at the moment, though (based on the coverage of this PR).

I'll have a deeper look at everything once the linter passes and these remarks are fixed.

pyproject.toml Outdated Show resolved Hide resolved
poetry.lock Outdated Show resolved Hide resolved
@lars-reimann lars-reimann marked this pull request as draft May 5, 2023 13:49
@lars-reimann lars-reimann changed the title feat: improve structure of table tests test: improve structure of table tests May 5, 2023
@lars-reimann lars-reimann marked this pull request as ready for review May 5, 2023 18:35
Copy link
Member

@lars-reimann lars-reimann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent work!

@lars-reimann lars-reimann merged commit 626f26b into main May 5, 2023
@lars-reimann lars-reimann deleted the 98-improve-structure-of-table-tests branch May 5, 2023 19:02
@lars-reimann
Copy link
Member

lars-reimann commented May 6, 2023

@daniaHu @jxnior01 @robmeth Please add the time you spent to issue #98.

@lars-reimann
Copy link
Member

🎉 This PR is included in version 0.12.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@lars-reimann lars-reimann added the released Included in a release label May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Included in a release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve structure of Table tests
5 participants