Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Configure document-title during updates #1315
Configure document-title during updates #1315
Changes from 1 commit
6525ca2
3061cba
eb427b0
28630d9
2f7cdc4
899b20e
f5f97cc
1db09c9
8046178
745ba77
57f9b37
740b913
71ee5ec
bb2a6f7
571d6ce
58b1472
087774d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Very nice tests! One thing though: your callback will execute on page load, right? So holding the lock while starting the server means we're testing the title during the initialization sequence. That's a good thing to test, but I think in at least one of these tests (and maybe all of them) we should test:
Also rather than a simple
assert
, for robustness I think it'd be better to do await.until
, in case things don't happen quite in the expected order:And one last thing, if you're feeling adventurous: These test cases are nearly identical so they could be parametrized, something like:
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.
I tried to implement your suggestions. I omitted an explicit test for the correct update title on startup because the callback should be called on startup afaik. I hope this is OK. The parametrize decorator is a great thing. Didn't know that until now, thanks :) Hope I did it all right with the locks. I'm not that sure about it.