kpi-web-guild / django-girls-blog-OlenaEfymenko

django-girls-blog-OlenaEfymenko created by GitHub Classroom
MIT License
1 stars 0 forks source link

Integrate coverage configuration and tests in CI pipeline #31

Closed OlenaYefymenko closed 8 months ago

OlenaYefymenko commented 10 months ago

This patch provides the coverage configuration, updates the project dependencies accordingly, and adds tests to the CI pipeline. This PR sets the stage for the next, where test functions will be integrated into the project.

Related to #18

OlenaYefymenko commented 10 months ago

@webknjaz I've implemented the coverage configuration as we did in my previous project. However, you mentioned an option regarding this:

#!/usr/bin/env python
"""Django's command-line utility for administrative tasks."""
import os
import sys

def main():
    os.environ.setdefault("DJANGO_SETTINGS_MODULE", "myproj.settings")

    # MyProject Customization: run coverage.py around tests automatically
    try:
        command = sys.argv[1]
    except IndexError:
        command = "help"

    running_tests = command == "test"
    if running_tests:
        from coverage import Coverage

        cov = Coverage()
        cov.erase()
        cov.start()

    try:
        from django.core.management import execute_from_command_line
    except ImportError as exc:
        raise ImportError(
            "Couldn't import Django. Are you sure it's installed and "
            "available on your PYTHONPATH environment variable? Did you "
            "forget to activate a virtual environment?"
        ) from exc
    execute_from_command_line(sys.argv)

    if running_tests:
        cov.stop()
        cov = Coverage()
        cov.save()
        covered = cov.report()
        if covered < 100:
            raise SystemExit(1)

if __name__ == "__main__":
    main()

I have doubts about whether I should keep the configuration as it is in the current PR or add it into manage.py using contextlib.nullcontext(). However, I've read that using a coveragerc file is preferable.

webknjaz commented 10 months ago

Use the coverage file for the configuration but integrate nullcontext for enabling coverage collection conditionally. Also, use the contextmanager decorator to move that to a separate helper function.

OlenaYefymenko commented 9 months ago

@webknjaz I've just added what we wrote in the live coding session.

webknjaz commented 8 months ago

I got some notification regarding a "truthy" yamllint rule and can't find that comment to see the context here. What was that about? Can you link it?

OlenaYefymenko commented 8 months ago

I got some notification regarding a "truthy" yamllint rule and can't find that comment to see the context here. What was that about? Can you link it?

Yes, first I pushed without this comment and CI linters crashed, but then I fixed this and deleted the comment

webknjaz commented 8 months ago

It's best to communicate what's happening. Deleting things only complicates everything.

OlenaYefymenko commented 8 months ago

Reminding to undo reshuffling the imports and address other comments.

I've merged the commits with the import into the previous, resolved conflicts. I hope this is appropriate for this issue. Also, I assume I've checked all comments, except for this one: https://github.com/kpi-web-guild/django-girls-blog-OlenaEfymenko/pull/31#discussion_r1425699563 Can we postpone addressing that one?