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

[bukuserver] New tab bug fix #659

Merged
merged 1 commit into from
Jan 7, 2023
Merged

Conversation

LeXofLeviafan
Copy link
Collaborator

fixes #658:

  • adding manual popup window check to close_if_popup() macro (instead of relying on browser check)

@rachmadaniHaryono
Copy link
Collaborator

@LeXofLeviafan
Copy link
Collaborator Author

https://github.com/jarun/buku/wiki/PR-guidelines#testing-bukuserver

The purpose of this section isn't clear to me. Is it meant for manual testing? It should be clarified in this case. (…And the list is kinda lengthy – it would make sense to go over those cases when preparing for a release, but not on a regular basis. There should be a way to automate at least some of those, too… Not the new tab thing tho, it's specific to builtin browser functionality.)

…Might want to mention navigation (cancel, save, save-and-add-another, save-and-continue-editing) for popup windows as well.

@rachmadaniHaryono
Copy link
Collaborator

Is it meant for manual testing? It should be clarified in this case. (…And the list is kinda lengthy – it would make sense to go over those cases when preparing for a release, but not on a regular basis. There should be a way to automate at least some of those, too… Not the new tab thing tho, it's specific to builtin browser functionality.)

yes it is manual testing for now. i think it is just as suggestion of what should be done before release.

new pr/feature may only test part of it.

for automation case, i don't know enough how to automate it. use selenium? also maybe check all links on bukuserver page for 404

new tab case is removed from that list

…Might want to mention navigation (cancel, save, save-and-add-another, save-and-continue-editing) for popup windows as well.

added on bookmarklet and bookmark creation test case

@jarun jarun merged commit 11cef65 into jarun:master Jan 7, 2023
@jarun
Copy link
Owner

jarun commented Jan 7, 2023

Thanks guys!

@LeXofLeviafan
Copy link
Collaborator Author

for automation case, i don't know enough how to automate it. use selenium? also maybe check all links on bukuserver page for 404

…I've looked up how Flask apps are meant to be tested, and ended up implementing a few functional tests. They don't cover the entire thing, of course (since they test server responses, not the client behaviour – e2e is more of a hassle to set up, and it's rather unstable), but they should at least halve the required manual testing. And I reckon there's a potential for more tests to be made (I mostly covered the bookmarklet popup, bookmark editing and configurable functionality), but I'm content with what I have at the moment 😅

@github-actions github-actions bot locked and limited conversation to collaborators Feb 9, 2023
@LeXofLeviafan LeXofLeviafan deleted the new-tab-fix branch August 24, 2024 17:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: can't use "open link in new tab" with navigation links
3 participants