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

Add unit tests for History View user script and fix reloading history #3855

Merged
merged 8 commits into from
Feb 10, 2025

Conversation

ayoy
Copy link
Collaborator

@ayoy ayoy commented Feb 10, 2025

Task/Issue URL: https://app.asana.com/0/72649045549333/1209365356928711

Description:
This change adds missing unit tests, adds event handling to History View (together with 2 pixels) and
fixes reloading history.

Steps to test this PR:

  1. Verify that CI is green
  2. Run the app from Xcode and enable historyView feature flag.
  3. Populate fake history items (Main Menu -> Debug -> History -> Add 10 history visits ...)
  4. Enter history (cmd+y or duck://history).
  5. Select an option from the left-hand side panel.
  6. Visit some website
  7. Reload history view. Verify that it properly reloaded data (putting the most recently visited website on top).

Definition of Done:


Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

Copy link
Contributor

github-actions bot commented Feb 10, 2025

Warnings
⚠️ PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 0a85249

@ayoy
Copy link
Collaborator Author

ayoy commented Feb 10, 2025

@jotaemepereira please note that HistoryViewDataProvider is not tested yet in this change. I'll provide tests for that class later. Thanks!

Copy link
Collaborator

@jotaemepereira jotaemepereira left a comment

Choose a reason for hiding this comment

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

@ayoy I’ve tested the feature and the unit tests looks good 👍🏼 .

@@ -27,7 +27,7 @@ extension Array {
}

func chunk(with limit: Int, offset: Int) -> [Element] {
guard !isEmpty, offset < count else {
guard !isEmpty, limit >= 0, offset >= 0, offset < count else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

❓ Did we have calls to the chunk method with negative values on limit and offset, is it something we should be worried about?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Only if accidentally called with a negative value, which shouldn't be the case, but this function is ultimately called from JS, so better safe than sorry :)

@ayoy ayoy merged commit e9867bc into release/1.126.0 Feb 10, 2025
24 of 30 checks passed
@ayoy ayoy deleted the dominik/history-view branch February 10, 2025 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants