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

Fix(CI): updated WizardTest to handle new email-validator error message #480

Merged
merged 2 commits into from
Jun 20, 2024

Conversation

SamuelLarkin
Copy link
Collaborator

@SamuelLarkin SamuelLarkin commented Jun 20, 2024

PR Goal?

email-validator has a new version and it changed one of its error message which we test in WizardTest.
Update the everyvoice.tests.test_wizard.WizardTest to handle both error messages.

Fixes?

Feedback sought?

Approval for merging.

Priority?

high

Tests added?

None

How to test?

pip install --upgrade email-validator
python -m unittest everyvoice.tests.test_wizard.WizardTest

Confidence?

High

Version change?

No but people will need to update their environment using

pip install --upgrade email-validator

Related PRs?

@joanise pointed out the possibility of a dependency having been updated causing the error in the first place.

@SamuelLarkin SamuelLarkin changed the title feat(CI): logged all python dependencies [WIP] Fix(CI): email-validator has changed Jun 20, 2024
Copy link
Contributor

github-actions bot commented Jun 20, 2024

CLI load time: 0:00.24
Pull Request HEAD: a2b0a1516b02fd804bb73c81918eea4db75f9068
Imports that take more than 0.1 s:
import time: self [us] | cumulative | imported package

Copy link

codecov bot commented Jun 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.59%. Comparing base (a050b99) to head (57dbcab).
Report is 1 commits behind head on main.

Current head 57dbcab differs from pull request most recent head a2b0a15

Please upload reports for the commit a2b0a15 to get more accurate results.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #480      +/-   ##
==========================================
- Coverage   74.30%   73.59%   -0.71%     
==========================================
  Files          45       45              
  Lines        2837     2886      +49     
  Branches      435      447      +12     
==========================================
+ Hits         2108     2124      +16     
- Misses        642      675      +33     
  Partials       87       87              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SamuelLarkin SamuelLarkin changed the title [WIP] Fix(CI): email-validator has changed Fix(CI): updated WizardTest to handle new email-validator error message Jun 20, 2024
Copy link
Member

@roedoejet roedoejet left a comment

Choose a reason for hiding this comment

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

lgtm - thanks @SamuelLarkin !

@@ -35,6 +35,9 @@ jobs:
pip install cython
pip install -e .
pip install coverage
- name: Log complete list of packages
run: |
pip freeze
Copy link
Member

Choose a reason for hiding this comment

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

good idea

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

While debugging this, I've noticed that not all dependencies are listed in Install dependencies and package. pip freeze list them all in a nicer and more concise manner.

@@ -227,7 +227,11 @@ def test_bad_contact_email_step(self):
self.assertTrue(step.validate("[email protected]"))
self.assertFalse(step.validate(""))
output = stdout.getvalue()
self.assertIn("It must have exactly one @-sign", output)
# Supporting email-validator prior and post 2.2.0 where the error string changed.
Copy link
Member

Choose a reason for hiding this comment

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

nice catch!

@SamuelLarkin SamuelLarkin merged commit d40429b into main Jun 20, 2024
2 checks passed
@SamuelLarkin SamuelLarkin deleted the dev.sl/email-validator branch June 20, 2024 21:30
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.

None yet

2 participants