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.

Sign up for GitHub

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

Jump to bottom

build: convert pact verification to unit test framework method #325

Merged
merged 1 commit into from
Nov 8, 2021
Merged

build: convert pact verification to unit test framework method #325

merged 1 commit into from
Nov 8, 2021

Conversation

DawoudSheraz
Copy link
Contributor

PROD-2562

Description

  • Convert edx-val pact provider verification in unit testing framework method to make it easier to integrate into CI
  • Usage of environment variable in setting some pact configuration in test.py file
  • Updating pytest to use test settings instead of base

@codecov-commenter
Copy link

codecov-commenter commented Nov 4, 2021 •
edited
Loading

Codecov Report

Merging #325 (b3d1c2a) into master (0ea91a6) will decrease coverage by 0.82%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #325      +/-   ##
==========================================
- Coverage   95.35%   94.53%   -0.83%     
==========================================
  Files          25       27       +2     
  Lines        2971     2999      +28     
  Branches      160      161       +1     
==========================================
+ Hits         2833     2835       +2     
- Misses        120      146      +26     
  Partials       18       18
Flag Coverage Δ
unittests 94.53% <0.00%> (-0.83%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
edxval/pacts/middleware.py 0.00% <0.00%> (ø)
edxval/pacts/verify_pact.py 0.00% <0.00%> (ø)
edxval/pacts/views.py 0.00% <0.00%> (ø)
edxval/settings/pact.py 0.00% <0.00%> (ø)
edxval/settings/test.py 100.00% <ø> (+100.00%) ⬆️

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 0ea91a6...b3d1c2a. Read the comment docs.

azanbinzahid
azanbinzahid reviewed Nov 4, 2021
View reviewed changes
setup.cfg
@@ -47,6 +47,6 @@ skip=
edxval/wsgi.py

[tool:pytest]
DJANGO_SETTINGS_MODULE = edxval.settings.base
DJANGO_SETTINGS_MODULE = edxval.settings.test
Copy link
Contributor

Choose a reason for hiding this comment

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

How are we providing django settings settings while running pact with pytest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

like we did with LMS, by exporting the settings module env variable before executing the verify call.

@Ali-D-Akbar
Copy link
Contributor

Can you update the readme for pact verification process with the updated steps to run?

Ali-D-Akbar
Ali-D-Akbar approved these changes Nov 5, 2021
View reviewed changes
Copy link
Contributor

@Ali-D-Akbar Ali-D-Akbar left a comment

Choose a reason for hiding this comment

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

Forgot to confirm this review before. Just a question, the rest looks good to me.

edxval/settings/pact.py Outdated
PUBLISH_VERIFICATION_RESULTS = os.environ.get('PUBLISH_VERIFICATION_RESULTS', 'False').lower() in ('true', 'yes', '1')
PROVIDER_STATES_SETUP_VIEW_URL = True
PROVIDER_BASE_URL = 'http://127.0.0.1:8000'
PROVIDER_STATES_URL = '{}/edxval/pact/provider_states/'.format(PROVIDER_BASE_URL)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we using this anywhere anymore? If not, this can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, nice catch.

@DawoudSheraz DawoudSheraz merged commit f8afe5f into master Nov 8, 2021
@DawoudSheraz DawoudSheraz deleted the dsheraz/PROD-2562 branch November 8, 2021 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ansabgillani ansabgillani ansabgillani approved these changes

@Ali-D-Akbar Ali-D-Akbar Ali-D-Akbar approved these changes

@azanbinzahid azanbinzahid azanbinzahid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@DawoudSheraz @codecov-commenter @Ali-D-Akbar @azanbinzahid @ansabgillani