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

Update pytest entrypoints #14868

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Conversation

janbrasna
Copy link
Contributor

@janbrasna janbrasna commented Jul 23, 2024

One-line summary

The py.test cli command is a legacy from times when pytest package was part of pylib — while it is godfathered to work and tests are in place to ensure entrypoint equivalency, it's nonetheless deprecated and only "pytest" should be used going forward.

Significant changes and points to review

The commands are equivalent for several years / major versions so this has no impact.

Issue / Bugzilla link

#14851

Testing

(docker-compose run test) pytest bedrock/firefox
is the same as
(docker-compose run test) py.test bedrock/firefox

also

mkdir test_infra
mkdir test_infra/fixtures
make test_infra/fixtures/tls.json
make test-cdn  

Copy link

codecov bot commented Jul 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.38%. Comparing base (5fd5d4f) to head (fbc9a13).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #14868   +/-   ##
=======================================
  Coverage   77.38%   77.38%           
=======================================
  Files         161      161           
  Lines        8305     8306    +1     
=======================================
+ Hits         6427     6428    +1     
  Misses       1878     1878           

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

Copy link
Collaborator

@stevejalim stevejalim left a comment

Choose a reason for hiding this comment

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

LGTM!

@janbrasna janbrasna marked this pull request as ready for review July 24, 2024 10:14
@stevejalim stevejalim merged commit 23c6e08 into mozilla:main Jul 24, 2024
5 checks passed
@janbrasna janbrasna deleted the upd/pytest-entrypoint branch July 24, 2024 10:19
@janbrasna
Copy link
Contributor Author

janbrasna commented Jul 24, 2024

@stevejalim I only wanted to check the CDN tests too, but wasn't able to figure out what I'm missing to successfully run them (maybe it's something set in ENV in addition to what I see?), even with the tls json bootstrapped I still get "0 tests to run" 🤷‍♂️ — will have to look deeper in the pipelines +their defs. I mean, there's no reason for anything to fail, it's all equivalent, but the only two legacy entrypoints were in places I wasn't able to test — Saucelabs and CDN… so that didn't really make me too comfortable;)

I'll keep an eye on checks off of main now, to see it getting run in the pipelines live. (BTW thanks for the runs! Was about to ask, appreciate it;))

@stevejalim
Copy link
Collaborator

stevejalim commented Jul 24, 2024

No worries, Jan - the CDN tests are fiddly to run locally, so I've generally evaluated them on main. I've just kicked off a run now - https://github.com/mozilla/bedrock/actions/runs/10075187548 - and will check on it after my next task.

And as you can see from https://github.com/mozilla/bedrock/actions/workflows/cdn_tests.yml the CDN tests aren't always stable due to downstream services (eg HTTP request failed: Get https://api.ssllabs.com/api/v2/analyze?host=https://www.mozilla.org&all=done: so if the run above goes red, it doesn't necessarily mean a regression.

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