8000 Add test fixture ignore_translations_for_mock_domains by emontnemery · Pull Request #139235 · home-assistant/core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Add test fixture ignore_translations_for_mock_domains #139235

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Feb 25, 2025
Merged

Add test fixture ignore_translations_for_mock_domains #139235

merged 6 commits into from
Feb 25, 2025

Conversation

emontnemery
Copy link
Contributor

Proposed change

Add test fixture ignore_translations_for_mock_domains which should be used when ignoring missing translation keys for integrations which doesn't exist.

Also, rename the existing fixture ignore_translations to ignore_missing_translations and fail tests if it's used for an integration which exists.

Rationale:
When validating translations, the translation cache is populated. This is not a problem for real integrations, but for mocked integrations this means tests may interfere with each other if they test behavior related to translations.

When validating translations, we will not load translations for integrations added to ignore_translations_for_mock_domains to prevent this.

This fixes flaky test tests/test_test_fixtures.py::test_evict_faked_translations

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)
  • Deprecation (breaking change to happen in the future)
  • 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:
  • Link to developer documentation pull request:
  • Link to frontend 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
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format 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.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@MartinHjelmare
Copy link
Member

I think the PR description is not completely accurate since also real integrations can be affected if a mock integration loads the cache with bad data and nothing is done about that after the test of the mock integration is done.

@emontnemery
Copy link
Contributor Author

Tests failing because a race in the loader was uncovered by the changes, fix in PR #139270

MartinHjelmare
MartinHjelmare approved these changes Feb 25, 2025
View reviewed changes
@emontnemery
Copy link
Contributor Author

I think the PR description is not completely accurate since also real integrations can be affected if a mock integration loads the cache with bad data and nothing is done about that after the test of the mock integration is done.

Yes, you're right, I think 👍
We do evict translations from mock integrations with help of the evict_faked_translations fixture so it's less likely that mocked integrations used in a test would interfere with some completely unrelated test of the real integration.

@MartinHjelmare

This comment was marked as outdated.

Sign in to view
@MartinHjelmare
Copy link
Member

I think the PR description is not completely accurate since also real integrations can be affected if a mock integration loads the cache with bad data and nothing is done about that after the test of the mock integration is done.

Yes, you're right, I think 👍 We do evict translations from mock integrations with help of the evict_faked_translations fixture so it's less likely that mocked integrations used in a test would interfere with some completely unrelated test of the real integration.

Yes, with that action we're addressing that problem.

@MartinHjelmare MartinHjelmare merged commit 51c09c2 into dev Feb 25, 2025
46 checks passed
@MartinHjelmare MartinHjelmare deleted the improve_validate_translation_fixtures branch February 25, 2025 19:10
@github-actions github-actions bot locked and limited conversation to collaborators Feb 26, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MartinHjelmare MartinHjelmare MartinHjelmare approved these changes

@fabaff fabaff Awaiting requested review from fabaff fabaff is a code owner

@gjohansson-ST gjohansson-ST Awaiting requested review from gjohansson-ST gjohansson-ST is a code owner

@Quentame Quentame Awaiting requested review from Quentame Quentame is a code owner

@mib1185 mib1185 Awaiting requested review from mib1185 mib1185 is a code owner

Assignees
No one assigned
Labels
cla-signed code-quality core has-tests small-pr PRs with less than 30 lines.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emontnemery @MartinHjelmare
0