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

test: add provider verification CI job #326

Merged
merged 3 commits into from
Nov 10, 2021
Merged

Conversation

Ali-D-Akbar
Copy link
Contributor

@Ali-D-Akbar Ali-D-Akbar commented Nov 8, 2021

PROD-2552

Adds a separate pact provider verification job in CI. The pacts are retrieved from pactflow, verification is performed and after that, the results are published with dev/prod tags and provider version.
Also, we're allowing Pending Pacts in edx-val as not doing that may block the edxval from merging the new changes if one or more consumers pacts are failing (even when the provider is compatible with the deployed version of the consumer)

@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2021

Codecov Report

Merging #326 (cdcc32c) into master (0188de1) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #326      +/-   ##
==========================================
- Coverage   94.59%   94.56%   -0.04%     
==========================================
  Files          27       27              
  Lines        2997     2998       +1     
  Branches      161      161              
==========================================
  Hits         2835     2835              
- Misses        144      145       +1     
  Partials       18       18              
Flag Coverage Δ
unittests 94.56% <0.00%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
edxval/pacts/verify_pact.py 0.00% <ø> (ø)
edxval/settings/pact.py 0.00% <0.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 0188de1...cdcc32c. Read the comment docs.

Comment on lines 45 to 75
- name: Publish Provider Verification Results
if: matrix.python-version == '3.8' && matrix.toxenv == 'django32'
run: |
export PACT_BROKER_BASE_URL=https://edx.pactflow.io
export PUBLISH_VERSION=`git rev-parse --short HEAD`
export PUBLISH_VERIFICATION_RESULTS=true
pytest edxval/pacts/verify_pact.py --ds=edxval.settings.pact
Copy link
Contributor

Choose a reason for hiding this comment

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

This job should be run separately and not part of unit tests. In VEM, publish pact job is run with unit tests because the pact is created/updated during the unit test suite. The same is not true for verification. This job will read pact from the broker/local file and then run the verification.

@Ali-D-Akbar Ali-D-Akbar force-pushed the aakbar/PROD-2552 branch 3 times, most recently from 4512b92 to dfda6a7 Compare November 9, 2021 08:02


provider-verification:
name: Publish Provider Verification Results
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Pact Provider Verification. No need to add publish in start.

pip install -r requirements/ci.txt
pip install -r requirements/test.txt

- name: Publish Results
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: verify pacts

export PACT_BROKER_TOKEN=${{ secrets.PACT_FLOW_ACCESS_TOKEN }}
export PUBLISH_VERSION=`git rev-parse --short HEAD`
export PUBLISH_VERIFICATION_RESULTS=true
pytest edxval/pacts/verify_pact.py --ds=edxval.settings.pact
Copy link
Contributor

Choose a reason for hiding this comment

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

Add -s flag so that logs/console statements are visible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh. On local, using -s yielded in a lot more log output, so I thought it was giving more output. I didn't consult the documents.

Copy link
Contributor

@DawoudSheraz DawoudSheraz left a comment

Choose a reason for hiding this comment

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

  • To add a link to Pending pacts document in PR description and why it is enabled

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.

4 participants