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

Prepare DB testing tools and data in dev dockerfile #900

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tai271828
Copy link
Member

Types of changes

  • Bugfix
  • New feature
  • Refactoring
  • Breaking change (any change that would cause existing functionality to not work as expected)
  • Documentation Update
  • Other (please describe)

Description

A begging movement for issue #891.

Steps to Test This Pull Request

Steps to reproduce the behavior:

  1. Build the dev docker containers with ./enter_dev_env.sh under contrib. (refer to contrib/dev_env.md for more details about how to build dev containers with docker-compose)
  2. Go to the landing page after the containers are ready.
  3. Go to the job listings page (of the event item of menu)

Expected behavior

  1. In the landing page you should see 2 fake sponsors (stacy ltd. and fake pycontw)
  2. In the landing page you should see the corresponding open roles of the above 2 fake sponsors.
  3. the end of the output message of docker-compose (note the line Inject DB testing data)
  Applying users.0010_cocrecord... OK
  Applying users.0011_cocrecord_agreed_at... OK
Create super user
Superuser created successfully.
Inject DB testing data
Installed 179 object(s) (of 180) from 1 fixture(s)
Compile localized translation
processing file django.po in /app/src/locale/en_US/LC_MESSAGES
processing file django.po in /app/src/locale/_src/LC_MESSAGES
processing file django.po in /app/src/locale/zh_Hant/LC_MESSAGES
[28/Aug/2020 10:21:39] I Watching for file changes with StatReloader
[28/Aug/2020 10:21:39] I Watching for file changes with StatReloader
Performing system checks...

Related Issue

#891 ## More Information

Screenshots
The media image files not presented are expected. We may commit the binary image files or big svg files later when we agree with each other to use this docker entry point to inject DB testing data for easier and more friendly code review process.

Selection_026

Selection_025

Additional context
This pull request does not provide a comprehensive solution for DB testing. Apart from that, it kicks off the whole story of issue #891 .

@codecov-commenter
Copy link

Codecov Report

Merging #900 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #900   +/-   ##
=======================================
  Coverage   76.21%   76.21%           
=======================================
  Files          79       79           
  Lines        2930     2930           
=======================================
  Hits         2233     2233           
  Misses        697      697           

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 8624991...299123d. Read the comment docs.

@shunghsiyu
Copy link
Member

shunghsiyu commented Aug 28, 2020

+1 on this, makes creating new page much easier.

I think db-testing-data.json can be pruned, right now it's 2000+ lines and contains mostly data not related to page content (e.g. admin, auth, session). IIRC you can export just some of the models manage.py dumpdata sponsor.

@@ -19,6 +19,7 @@ RUN apt-get update
RUN apt-get install apt-utils -y
RUN apt-get update
RUN apt-get install gettext python3-pip -y
RUN apt-get install postgresql-client -y
Copy link
Member

@shunghsiyu shunghsiyu Aug 28, 2020

Choose a reason for hiding this comment

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

A bit of nitpicking, but strictly speaking the client is not required since you can just docker exec -it $db_container psql to use the postgresql client that's inside the database container.

However this does make development much friendlier. Maybe add this in a separate commit instead so it's not implied that postgresql-client is required to run manage.py loaddata?

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.

3 participants