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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve test coverage #1372

Conversation

matthewhegarty
Copy link
Contributor

@matthewhegarty matthewhegarty commented Dec 26, 2021

Problem

For inclusion in Release 3

Code coverage seems to miss certain paths, meaning coverage is lower than it should be.
This is because the IMPORT_EXPORT_TEST_TYPE env var is not being set correctly for test runs, which means that postgres / mysql specific tests are not being run as part of the CI process.

Solution

I switched the CI script to use tox instead of the github actions matrix. This is a better approach because tox is already present in the project, though not being used during CI.

  • Fixed environment variables in the CI scripts.
  • Added extra unit tests to cover lines being missed in test coverage.
  • Removed default username and password from MySQL test db for security best practices (settings.py).
  • Added deprecation warning to unused exceptions.py module (and added appropriate test)
  • Removed testing against tablibdev for every test run - this seems overkill - so now there is a single test for tablib dev for each test run.
  • Moved 'tests/bulk/docker*' to tests
  • Updated runtests.sh - now will run tox tests for each supported db (sqlite, postgres, MySQL)
  • Added new documentation page testing.rst
  • Added MySQL to docker-compose.yml to support testing
  • CI builds are also around 5x faster 馃樅
  • Code coverage 100% 馃殌

Acceptance Criteria

  • Check that Postgres / MySQL specific tests are run (visible in the test output).
  • Test coverage increases.

@coveralls
Copy link

coveralls commented Dec 26, 2021

Coverage Status

Coverage increased (+0.2%) to 98.309% when pulling 76fe540 on matthewhegarty:test-coverage-update into 0d4340f on django-import-export:main.

@matthewhegarty matthewhegarty changed the title improve coverage collection improve test coverage Dec 26, 2021
matthewhegarty added a commit to matthewhegarty/django-import-export that referenced this pull request Dec 26, 2021
@matthewhegarty matthewhegarty changed the base branch from main to release-3-x December 26, 2021 22:57
@matthewhegarty matthewhegarty self-assigned this Dec 27, 2021
matthewhegarty added a commit to matthewhegarty/django-import-export that referenced this pull request Dec 27, 2021
matthewhegarty added a commit to matthewhegarty/django-import-export that referenced this pull request Dec 27, 2021
matthewhegarty added a commit to matthewhegarty/django-import-export that referenced this pull request Dec 27, 2021
matthewhegarty added a commit to matthewhegarty/django-import-export that referenced this pull request Dec 28, 2021
matthewhegarty added a commit to matthewhegarty/django-import-export that referenced this pull request Dec 29, 2021
@matthewhegarty matthewhegarty marked this pull request as ready for review December 29, 2021 13:13
docs/changelog.rst Outdated Show resolved Hide resolved
tests/bulk/README.md Show resolved Hide resolved
docs/testing.rst Outdated Show resolved Hide resolved
docs/changelog.rst Show resolved Hide resolved
docs/testing.rst Outdated Show resolved Hide resolved
tests/docker-compose.yml Show resolved Hide resolved
@matthewhegarty
Copy link
Contributor Author

@manelclos many thanks for reviewing - I've added some comments and made some changes

Copy link
Contributor

@manelclos manelclos left a comment

Choose a reason for hiding this comment

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

LGTM

@matthewhegarty matthewhegarty merged commit 5b3b250 into django-import-export:release-3-x Dec 29, 2021
@matthewhegarty matthewhegarty deleted the test-coverage-update branch December 29, 2021 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants