-
Notifications
You must be signed in to change notification settings - Fork 1
Fix game.play and its test #7
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
base: master
Are you sure you want to change the base?
Conversation
1. One of the test's assertion was missing the `assert`. 2. Adding the above `assert` revealed that text is translated before it is formatted. So I fixed it to translate only after formatting it. (Because formats can have variable name in them, and we don't want them translated)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 👍
- see comments in my review.
- (Also) the
BytesIO
you added broke the tests in Python 3.
I guess that for Python 2 it was needed, so I put it where I thought it should be, but since I didn't test it The code was left commented out. If your requires it, please uncomment and fix as you see fit.
print(T("Level {level}: 0 to {max_num}").format( | ||
level=level, max_num=max_number(level))) | ||
print(T("Level {level}: 0 to {max_num}".format( | ||
level=level, max_num=max_number(level)))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, when the implementation of T
is not as stupid as I did it, the original is the correct line. Here how it suppose to work:
Some pre-processor should run on this code and collect all the T("...")
lines and put all the strings into a .pot
file (portable object template). Then human translators (with the aid of a dedicated s/w) will translate each string to their target languages, creating a set of .po
files (portable object). Our s/w will pick the correct file according the the system's local, and use the string in T("...") as a key to find the translated string in that file, and use it.
So as a result you need to keep the {...}
in the call to T
and the translator need to keep them (although she might need to move them around, if the target language requires it). If we will not do this, there might be infinite strings to translate.
I appreciate that a machine translate might muck things here, but consider the case where we want to call the translate service only once and cache the results.
guess = your_guess() | ||
if guess is None: | ||
print(T("Just wanted you to know that I was thinking about {}").format(num)) | ||
print(T("Just wanted you to know that I was thinking about {}".format(num))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above.
game.play(level) | ||
|
||
mock_T.call_args_list = map(mock.call, expected_messages) | ||
assert mock_T.call_args_list == map(mock.call, expected_messages) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
assert
.assert
revealed that text is translated before it is formatted.So I fixed it to translate only after formatting it.
(Because formats can have variable name in them, and we don't want them translated)
Also, I'm not sure what you did with BytesIO that I tried to introduce, but I can't run the tests without it. Care to explain? @chenl