The Wayback Machine - https://web.archive.org/web/20210724220405/https://github.com/home-assistant/core/pull/53203
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

Making Pytest default for VS code #53203

Merged
merged 2 commits into from Jul 22, 2021
Merged

Conversation

@chemelli74
Copy link
Contributor

@chemelli74 chemelli74 commented Jul 19, 2021

Proposed change

This small change will allow a user to run pytest inside VScode and attach to the debugger to see what's going on.
Thx to @Santobert for pointing me to it!

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@frenck
Copy link
Member

@frenck frenck commented Jul 19, 2021

The VSCode extension already provides this?

I can click on run/run with debug on every individual test right from the IDE in VSCode...

@chemelli74
Copy link
Contributor Author

@chemelli74 chemelli74 commented Jul 19, 2021

The VSCode extension already provides this?

I can click on run/run with debug on every individual test right from the IDE in VSCode...

I may miss VS extension then :-(
Which one are you using ?

Simone

@chemelli74
Copy link
Contributor Author

@chemelli74 chemelli74 commented Jul 19, 2021

The VSCode extension already provides this?
I can click on run/run with debug on every individual test right from the IDE in VSCode...

I may miss VS extension then :-(
Which one are you using ?

Simone

Got it:

  • From command palette run "Task: Configure Default Test Task"
  • From command palette run "Python: Discover Tests"
  • Now you have the ability to run each test from UI :
    image

Simone

@chemelli74 chemelli74 changed the title Allow easy run of pytest with debugger Making Pytest default for VS code Jul 19, 2021
@frenck
Copy link
Member

@frenck frenck commented Jul 20, 2021

I'm confused about what this PR does at this point, could you explain the difference?

@chemelli74
Copy link
Contributor Author

@chemelli74 chemelli74 commented Jul 20, 2021

I'm confused about what this PR does at this point, could you explain the difference?

First I was thinking about adding the pytest in the launch.json section following @Santobert idea.

Once I learned that there is a better way to proceed (thx for that), I though that we should put pytest as the default test task so the discover tests works out of the box.

Simone

@frenck
Copy link
Member

@frenck frenck commented Jul 20, 2021

I think the problem with that (if I remember right) is will always kick off the discovery when you set up / open the environment. This is quite a resource/CPU-intensive operation and I believe Paulus disabled that for that reason.

(I might be wrong, digging my possible faulty memory here)

@chemelli74
Copy link
Contributor Author

@chemelli74 chemelli74 commented Jul 21, 2021

I think the problem with that (if I remember right) is will always kick off the discovery when you set up / open the environment. This is quite a resource/CPU-intensive operation and I believe Paulus disabled that for that reason.

(I might be wrong, digging my possible faulty memory here)

Seems current version of VS code and Python extension don't trigger a rescan at each start.
Maybe someone else can check and confirm.

If otherwise, we can close this PR and just add some info to the docs.

Simone

@frenck
Copy link
Member

@frenck frenck commented Jul 21, 2021

I'm ok with this personally, @balloob what do you think?

@frenck frenck requested a review from balloob Jul 21, 2021
@frenck
frenck approved these changes Jul 21, 2021
Dev automation moved this from Needs review to Reviewer approved Jul 21, 2021
@balloob
Copy link
Member

@balloob balloob commented Jul 21, 2021

Yeah I had indeed the issue that it would trigger a re-discover on each save of a file. It also requires to have all requirements installed or else it wouldn't run.

I just tried this PR and it is indeed no longer doing this when I hit save. Discovery still crashes because I don't have all requirement installed.

However, I still think that running Home Assistant should be the default task. It's kind shouldn't be "test" maybe, but having it start Home Assistant when hitting play is what I would expect when having a dev environment for an app open.

@chemelli74
Copy link
Contributor Author

@chemelli74 chemelli74 commented Jul 21, 2021

I just tried this PR and it is indeed no longer doing this when I hit save. Discovery still crashes because I don't have all requirement installed.

However, I still think that running Home Assistant should be the default task. It's kind shouldn't be "test" maybe, but having it start Home Assistant when hitting play is what I would expect when having a dev environment for an app open.

I think we are using different task/menu entries then ;-)
If i do a F5 or I go in the left bar and go to the "Run and debug", I always find HomeAssistant as the default entry selected.

What di PR does is changing the Default Test Task making, not the main debug task.

Do I miss something ?

Simone

@balloob
Copy link
Member

@balloob balloob commented Jul 22, 2021

If that's the case, I am fine with merging this 👍

@balloob balloob merged commit b2528e9 into home-assistant:dev Jul 22, 2021
30 checks passed
30 checks passed
@github-actions
Prepare base dependencies
Details
@github-actions
Prepare tests for Python 3.8
Details
@github-actions
Prepare tests for Python 3.9
Details
@github-actions
Check bandit
Details
@github-actions
Check black
Details
@github-actions
Check codespell
Details
@github-actions
Check Dockerfile
Details
@github-actions
Check executables
Details
@github-actions
Check flake8
Details
@github-actions
Check isort
Details
@github-actions
Check JSON
Details
@github-actions
Check pyupgrade
Details
@github-actions
Check YAML
Details
@github-actions
Check all requirements
Details
@github-actions
Check hassfest (3.8)
Details
@github-actions
Check pylint (3.8)
Details
@github-actions
Check mypy (3.8)
Details
@github-actions
Run tests Python 3.8 (group 1)
Details
@github-actions
Run tests Python 3.9 (group 1)
Details
@github-actions
Run tests Python 3.8 (group 2) Run tests Python 3.8 (group 2)
Details
@github-actions
Run tests Python 3.9 (group 2) Run tests Python 3.9 (group 2)
Details
@github-actions
Run tests Python 3.8 (group 3)
Details
@github-actions
Run tests Python 3.9 (group 3)
Details
@github-actions
Run tests Python 3.8 (group 4)
Details
@github-actions
Run tests Python 3.9 (group 4)
Details
@github-actions
Process test coverage (3.8)
Details
@homeassistant
cla-bot Everyone involved has signed the CLA
@codecov
codecov/patch Coverage not affected when comparing 450fdc9...38e349d
Details
@codecov
codecov/project 96.46% (target 90.00%)
Details
@probot-home-assistant
docs-missing Documentation ok.
Dev automation moved this from Reviewer approved to Done Jul 22, 2021
@chemelli74 chemelli74 deleted the chemelli74:chemelli74-debug branch Jul 22, 2021
raman325 added a commit to raman325/home-assistant that referenced this pull request Jul 22, 2021
* upstream/dev: (229 commits)
  Use entity class attributes for canary (home-assistant#53333)
  Update to PyVicare 1.0 (home-assistant#53281)
  Do not automatically add title to strings.json (home-assistant#53350)
  Replace util.get_local_ip in favor of components.network.async_get_source_ip() - part 1 (home-assistant#52980)
  Create APIs for Insteon panel (home-assistant#49785)
  Bump aionotion to 3.0.2 (home-assistant#53354)
  Handle more Sonos snapshot restore scenarios (home-assistant#53277)
  Making Pytest default for VS code (home-assistant#53203)
  Fix for issue home-assistant#53031 (home-assistant#53343)
  Add state class and last reset to consumption sensor in devolo Home Control (home-assistant#53337)
  Fritzbox enable temp sensor (home-assistant#52558)
  Upgrade wled to 0.7.3 (home-assistant#53340)
  Use NamedTuple - rainbird (home-assistant#53329)
  Use entity class attributes for caldav (home-assistant#53332)
  Add Automate Pulse Hub v2 support (home-assistant#39501)
  Switch wirelesstag to use cloud push (home-assistant#50984)
  Remove energy attributes from switch platform in devolo Home Control (home-assistant#53335)
  Add a homekit.unpair service to forcefully remove pairings (home-assistant#53303)
  Use NamedTuple - rova (home-assistant#53292)
  Speedtestdotnet code cleanup and type hints (home-assistant#52533)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants