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

tests: update smtp unit tests #179

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

joseph-sentry
Copy link
Contributor

@joseph-sentry joseph-sentry commented Nov 9, 2023

some smtp unit tests are broken in ATS sometimes, this PR attempts to fix them

@codecov-staging
Copy link

codecov-staging bot commented Nov 9, 2023

Codecov Report

Merging #179 (820941f) into main (a873c92) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #179   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         350      350           
  Lines       27721    27721           
=======================================
  Hits        27274    27274           
  Misses        447      447           
Flag Coverage Δ
integration 98.38% <100.00%> (ø)
latest-uploader-overall 98.38% <100.00%> (ø)
unit 98.38% <100.00%> (ø)

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

Components Coverage Δ
NonTestCode 96.88% <ø> (ø)
OutsideTasks 98.17% <100.00%> (ø)
Files Coverage Δ
services/tests/test_smtp.py 100.00% <100.00%> (ø)

@codecov-qa
Copy link

codecov-qa bot commented Nov 9, 2023

Codecov Report

Merging #179 (820941f) into main (a873c92) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #179   +/-   ##
=======================================
  Coverage   98.38%   98.38%           
=======================================
  Files         350      350           
  Lines       27721    27721           
=======================================
  Hits        27274    27274           
  Misses        447      447           
Flag Coverage Δ
integration 98.38% <100.00%> (ø)
latest-uploader-overall 98.38% <100.00%> (ø)
unit 98.38% <100.00%> (ø)

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

Components Coverage Δ
NonTestCode 96.88% <ø> (ø)
OutsideTasks 98.17% <100.00%> (ø)
Files Coverage Δ
services/tests/test_smtp.py 100.00% <100.00%> (ø)

Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #179 (820941f) into main (a873c92) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #179   +/-   ##
=======================================
  Coverage   98.34%   98.34%           
=======================================
  Files         376      376           
  Lines       28217    28217           
=======================================
  Hits        27751    27751           
  Misses        466      466           
Flag Coverage Δ
integration 98.38% <100.00%> (ø)
latest-uploader-overall 98.38% <100.00%> (ø)
onlysomelabels 97.63% <0.00%> (-0.01%) ⬇️
unit 98.38% <100.00%> (ø)

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

Components Coverage Δ
NonTestCode 96.77% <ø> (ø)
OutsideTasks 98.17% <100.00%> (ø)
Files Coverage Δ
services/tests/test_smtp.py 100.00% <100.00%> (ø)

This change has been scanned for critical changes. Learn more

@joseph-sentry joseph-sentry changed the title update smtp unit tests tests: update smtp unit tests Nov 9, 2023
Copy link
Contributor

@giovanni-guidini giovanni-guidini left a comment

Choose a reason for hiding this comment

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

Thanks for doing this.
I'll approve cause you say it's to fix tests in ATS, and this run is green for it, so seems ok. The PR comment or the comment don't give much info on the issue itself...

This commit adds the reset_connection fixture
to some tests that were broken in the ATS CI step.

The reason they were broken is because the order of
the tests being ran was hiding this issue of reusing the
existing SMTP connection across tests.

Signed-off-by: joseph-sentry <[email protected]>
@joseph-sentry
Copy link
Contributor Author

Thanks for doing this. I'll approve cause you say it's to fix tests in ATS, and this run is green for it, so seems ok. The PR comment or the comment don't give much info on the issue itself...

updated the commit message to try and make it more clear

@joseph-sentry joseph-sentry merged commit 0a1f963 into main Nov 14, 2023
24 checks passed
@joseph-sentry joseph-sentry deleted the joseph/update-smtp-tests branch November 14, 2023 16:28
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.

2 participants