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

Simplify asserts on translations #4359

Merged
merged 4 commits into from
Nov 25, 2024
Merged

Simplify asserts on translations #4359

merged 4 commits into from
Nov 25, 2024

Conversation

matejcik
Copy link
Contributor

with this change, instead of having to use a cumbersome custom assert_in assert_in_multiple etc. and mucking around with multiple possible translations
you instead use the normal assert capabilities, and say things like assert TR.some__translation_key in debug.text_content(), which any Python reader can decipher.

also pytest will point you directly to the failing assertion instead of showing you a problem in the function "assert_equals"

before:
image

after:
image

as a bonus, you're actually checking against the currently selected language, which caught a small bug in a couple tests, where language was not re-set after a wipe


  • for templates, use TR.regexp("translation__key").match(...)
  • if you don't know the literal TR key, e.g., caller supplies it as a string, you can use TR.translate(key) (or, you know, getattr(TR, key), but the former is preferable)
  • assert_in_multiple is now significantly wordier, because the operation "in multiple" does not exist in Python. personally I prefer this for clarity, but if it's a big problem we could re-introduce a helper for this situation?

Copy link

github-actions bot commented Nov 18, 2024

legacy UI changes device test(screens) main(screens)

Copy link

github-actions bot commented Nov 18, 2024

core UI changes device test click test persistence test
T2T1 Model T test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3B1 Safe 3 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
T3T1 Safe 5 test(screens) main(screens) test(screens) main(screens) test(screens) main(screens)
All main(screens)

@matejcik matejcik self-assigned this Nov 18, 2024
@matejcik matejcik added the translations Put this label on a PR to run tests in all languages label Nov 18, 2024
Copy link
Contributor

@obrusvit obrusvit left a comment

Choose a reason for hiding this comment

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

Nice improvement.

I tried to make LSP somehow search for these keys on auto-completion so that you would get hints while writing tests but I had no success.

Tried:

  • using __setattr__ in __init__
  • overriding __dir__

The older approach didn't provide any auto-completion support neither though.

tests/translations.py Show resolved Hide resolved
@matejcik
Copy link
Contributor Author

I tried to make LSP somehow search for these keys on auto-completion so that you would get hints while writing tests but I had no success.

we'd probably have to generate a file that has all the available arguments, similar to trezortranslate_keys.pyi

we need to propagate stacklevel so that the warning is emitted at the
usage line instead of deep in translations.py

i mean
we don't _need_ need to
it's just nicer
this changes UI tests because in edge cases the carousel goes the other
(shorter) way
@matejcik matejcik merged commit 50447de into main Nov 25, 2024
137 of 139 checks passed
@matejcik matejcik deleted the matejcik/tr-asserts branch November 25, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
translations Put this label on a PR to run tests in all languages
Projects
Status: 🤝 Needs QA
Development

Successfully merging this pull request may close these issues.

2 participants