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

Unit tests to cover sensitive areas of Burner Window #1169

Merged
merged 5 commits into from
May 14, 2023

Conversation

tomasstrba
Copy link
Contributor

@tomasstrba tomasstrba commented Apr 28, 2023

Task/Issue URL: https://app.asana.com/0/0/1204489493401944/f
BSK PR: duckduckgo/BrowserServicesKit#340

Description:
Unit tests to make sure Burner Windows don't store history, downloads and favicons.

Steps to test this PR:

  1. Do a smoke test of Burner Window
  2. Make sure tests are passing

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

@tomasstrba tomasstrba requested review from a team and diegoreymendez and removed request for a team April 28, 2023 16:12
Copy link
Contributor

@diegoreymendez diegoreymendez left a comment

Choose a reason for hiding this comment

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

Good work. Even though I'm no expert in this area of the code I understood what the tests are checking for and it seems good to me.

Additionally they build and run successfully.

class HistoryTabExtensionTests: XCTestCase {

@MainActor
func testWhenBurnerTab_ThenNoHistoryIsStored() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't feel strongly against this, but I just went ahead to check the code and noticed there aren't many instances of this so it made me wonder if it was on purpose.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Diego, that's a good question. I have also noticed we lack more tests for tab extensions.

kind = exactVersion;
version = 57.4.2;
branch = "tom/burner-window-unit-tests";
kind = branch;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a reminder to update this, since sometimes it's easy to forget.

@diegoreymendez
Copy link
Contributor

@tomasstrba - Just a reminder that this is good to go.

@tomasstrba tomasstrba merged commit f577b80 into develop May 14, 2023
@tomasstrba tomasstrba deleted the tom/burner-window-unit-tests branch May 14, 2023 13:08
samsymons added a commit that referenced this pull request May 23, 2023
* upstream_develop:
  sync device connected names (#1205)
  Autofill/subdomain matching (#1122)
  add-windows-browser-download-link-privacy-config (#1198)
  Update metadata
  Fix creating DMG from release workflow (#1200)
  Unit tests to cover sensitive areas of Burner Window (#1169)
  [Hackdays] Share via QR Code (#1177)
  Fix sync connect flow  (#1195)
  Update to BSK adding new remote message image (#1194)
  Set version to 1.39.0
  Update embedded files
  Update old Dax asset (#1193)
  WebKit find in page (#1188)
  modify manage bookmarks button (#1175)
  sync update device model / poll for devices (#1191)
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