Skip to content
This repository has been archived by the owner on Jan 26, 2021. It is now read-only.

feat: Github action integrated with codecov #632

Merged
merged 8 commits into from
Aug 22, 2020
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ DB_PASSWORD =
DB_HOST = localhost
DB_PORT = 5432

# Test DB settings

TEST_DB_NAME = systersdb
TEST_DB_USER = test
TEST_DB_PASSWORD = password
TEST_DB_HOST = localhost
TEST_DB_PORT = 5432

# Email settings

EMAIL_HOST = localhost
Expand Down
53 changes: 53 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: Run Tests

on:
push:
branches: [ develop ]
pull_request:
branches: [ develop ]

jobs:
build:
runs-on: ubuntu-18.04
strategy:
matrix:
python-version: [3.6, 3.7, 3.8]

services:
postgres:
image: postgres:10.8
env:
POSTGRES_USER: test
POSTGRES_PASSWORD: password
POSTGRES_DB: systersdb
ports:
- 5432:5432
# needed because the postgres container does not provide a healthcheck
options: --health-cmd pg_isready --health-interval 10s --health-timeout 5s --health-retries 5

steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v1
with:
python-version: ${{ matrix.python-version }}

- name: Install Dependencies
run: |
cp .env.example .env
python -m pip install --upgrade pip
pip install -r requirements/dev.txt
sudo apt-get install python-gdal
python -c "import nltk; nltk.download('punkt'); nltk.download('stopwords')"

- name: Run Flake Test
satya7289 marked this conversation as resolved.
Show resolved Hide resolved
run: flake8 systers_portal

- name: Run Tests
run: coverage run systers_portal/manage.py test --settings=systers_portal.settings.testing

- name: Upload coverage to Codecov
uses: codecov/codecov-action@v1
with:
name: codecov-umbrella
fail_ci_if_error: true
55 changes: 0 additions & 55 deletions .travis.yml

This file was deleted.

2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
Systers Portal [![Build Status](https://travis-ci.org/systers/portal.svg?branch=master)](https://travis-ci.org/systers/portal) [![Coverage Status](https://coveralls.io/repos/github/systers/portal/badge.svg?branch=master)](https://coveralls.io/r/systers/portal?branch=master)
Systers Portal [![Build Status](https://codecov.io/gh/anitab-org/portal/branch/develop/graph/badge.svg)](https://codecov.io/gh/anitab-org/portal/) [![Coverage Status](https://coveralls.io/repos/github/systers/portal/badge.svg?branch=master)](https://coveralls.io/r/systers/portal?branch=master)
==============

Systers Portal is for Systers communities to post and share information within
Expand Down
10 changes: 4 additions & 6 deletions systers_portal/meetup/tests/test_forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
from django.utils.timezone import timedelta
from cities_light.models import City, Country
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ValidationError


from meetup.forms import (AddMeetupForm, EditMeetupForm,
Expand Down Expand Up @@ -61,9 +60,9 @@ def test_request_meetup_form_with_past_date(self):
date = (timezone.now() - timedelta(2)).date()
time = timezone.now().time()
data = {'title': 'Foo', 'slug': 'foo', 'date': date, 'time': time,
'meetup_location': self.location,
'meetup_location': self.location.id,
'description': "It's a test meetup."}
form = AddMeetupForm(data=data, created_by=self.systers_user, leader=self.systers_user)
form = RequestMeetupForm(data=data, created_by=self.systers_user)
self.assertFalse(form.is_valid())
self.assertTrue(form.errors['date'], ["Date should not be before today's date."])

Expand All @@ -72,13 +71,12 @@ def test_request_meetup_form_with_passed_time(self):
date = timezone.now().date()
time = (timezone.now() - timedelta(2)).time()
data = {'title': 'Foo', 'slug': 'foo', 'date': date, 'time': time,
'meetup_location': self.location,
'meetup_location': self.location.id,
'description': "It's a test meetup."}
form = AddMeetupForm(data=data, created_by=self.systers_user, leader=self.systers_user)
form = RequestMeetupForm(data=data, created_by=self.systers_user)
self.assertFalse(form.is_valid())
self.assertTrue(form.errors['time'],
["Time should not be a time that has already passed."])
self.assertRaises(ValidationError, form.clean_time())
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand any of the changes here, and please don't remove a whole test check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you check deeply then you come to know that these are test cases for RequestMeetupFormTestCase not AddMeetupFormTestCase that's why I am adding the RequestMeetupForm instead of AddMeetupForm Actually whole test cases are wrong..

Also, Here this line self.assertRaises(ValidationError, form.clean_time()) is not needed because in the clean_time method time is used for return ValidationError(however, here time is None because on adding requestmeetup due to clean_time method time will be None). You can check these phenomenon by printing form.clean().

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for diving deep into this! Can you please open an issue about this?



class AddMeetupFormTestCase(MeetupFormTestCaseBase, TestCase):
Expand Down
10 changes: 5 additions & 5 deletions systers_portal/systers_portal/settings/testing.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
DATABASES = {
'default': {
'ENGINE': 'django.db.backends.postgresql',
'NAME': config('DB_NAME', default='systersdb'),
'USER': config('DB_USER'),
'PASSWORD': config('DB_PASSWORD'),
'HOST': config('DB_HOST', default='localhost'),
'PORT': config('DB_PORT', default=5432, cast=int),
'NAME': config('TEST_DB_NAME', default='systersdb'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: why not keep the same names?

So basically the same names for beth dev and testing, is that causing issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because I wanted to keep the test DB and main DB different. Actually, This TEST_DB is required for only Github's action databases.
However, This TEST_DB must be empty on the dev environment for running test suits locally.
@SanketDG

Copy link
Contributor

@SanketDG SanketDG Aug 22, 2020

Choose a reason for hiding this comment

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

I understand but this has no effect https://docs.djangoproject.com/en/3.1/topics/testing/overview/#the-test-database

I am pretty sure the config involved is wrong, I had raised the issue before in a weekly session.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whatever name you supply, test_ will be prepended.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Which config are you thinking wrong..! TEST_DB or anything else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can even use the SQLite database for testing purpose. If we use SQLite then no need to use configure extra services for PostgreSQL in Github Actions...!

Copy link
Contributor

Choose a reason for hiding this comment

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

TEST_DB yes!

There is a reason why Django uses the same engine by default, I'd like to use the same database as used for dev (unless we will have to go through a lot of obstacles), as there is no actual significant improvement to use a different database, but lot's of things can go wrong (databases have discrepancies between them)

What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please look at now, I have updated with SQLite testing DB. SQLite is a default database for Django.
I think there should no problem with using SQLite for testing purposes as far I know Django doesn't depend on which DB are you using.
Please let me know if anything went wrong if I use SQLite as testing DB.

Copy link
Contributor

Choose a reason for hiding this comment

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

Well, I am not finding a good reason to go for such a discrepancy of not using the same database.

I will be happy to merge this though.

Also, can you squash your commits to be the actual issues you worked on (Github Actions and 3.8 update), otherwise I will just squash into one commit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Squashing to 1 commit will be fine.

'USER': config('TEST_DB_USER'),
'PASSWORD': config('TEST_DB_PASSWORD'),
'HOST': config('TEST_DB_HOST', default='localhost'),
'PORT': config('TEST_DB_PORT', default=5432, cast=int),
}
}

Expand Down