Skip to content

Conversation

@tvalle
Copy link
Contributor

@tvalle tvalle commented Jan 7, 2021

Task/Issue URL: #879

Description:
It seems that what caused #879 was that after the tab is automatically closed BrowserChromeClient::onCloseWindow is called and BrowserTabViewModel::closeCurrentTab will then be called but removeCurrentTabFromRepository() uses tabRepository.delete(currentTab) instead of tabRepository.deleteCurrentTabAndSelectSource() (which fixes the issue).

Steps to test this PR:

  1. Have two tabs open, the second tab should have some sort of social login (I used canva.com).
  2. On the second tab login into your account.
  3. When the login finishes it must go back to the previous tab instead of the first one.

Internal references:

Software Engineering Expectations
Technical Design Template

@tvalle
Copy link
Contributor Author

tvalle commented Jan 7, 2021

Hi, I read the logs on bitrise but couldn't figure out which tests failed, could someone give me some pointers?

Locally there were 4 tests failing but they also failed before I made my changes, so I don't know exactly if I caused a regression.

@cmonfortep
Copy link
Contributor

cmonfortep commented Jan 7, 2021

Thanks @tvalle.

Here you have the unit tests that are failing, they should be failing in local too. I'm not aware that they were failing before but I will take a look.

whenUserClicksOnErrorActionThenOpenCurrentTabIsClosed

Wanted but not invoked:
mockTabsRepository.delete(
TabEntity(tabId=TAB_ID, url=https://example.com, title=, skipHome=false, viewed=true, position=0, tabPreviewFile=null, sourceTabId=null, deletable=false)
);
-> at com.duckduckgo.app.browser.BrowserTabViewModelTest$whenUserClicksOnErrorActionThenOpenCurrentTabIsClosed$1.invokeSuspend(BrowserTabViewModelTest.kt:1449)

whenInvalidatedGlobalLayoutAndNonEmptyInputThenCloseCurrentTab

Wanted but not invoked:
mockTabsRepository.delete(
TabEntity(tabId=TAB_ID, url=https://example.com, title=, skipHome=false, viewed=true, position=0, tabPreviewFile=null, sourceTabId=null, deletable=false)
);
-> at com.duckduckgo.app.browser.BrowserTabViewModelTest$whenInvalidatedGlobalLayoutAndNonEmptyInputThenCloseCurrentTab$1.invokeSuspend(BrowserTabViewModelTest.kt:544)

whenRefreshRequestedWithInvalidatedGlobalLayoutThenCloseCurrentTab

Wanted but not invoked:
mockTabsRepository.delete(
TabEntity(tabId=TAB_ID, url=https://example.com, title=, skipHome=false, viewed=true, position=0, tabPreviewFile=null, sourceTabId=null, deletable=false)
);
-> at com.duckduckgo.app.browser.BrowserTabViewModelTest$whenRefreshRequestedWithInvalidatedGlobalLayoutThenCloseCurrentTab$1.invokeSuspend(BrowserTabViewModelTest.kt:1199)

whenCloseCurrentTabSelectedThenTabDeletedFromRepository

Wanted but not invoked:
mockTabsRepository.delete(
TabEntity(tabId=TAB_ID, url=https://example.com, title=, skipHome=false, viewed=true, position=0, tabPreviewFile=null, sourceTabId=null, deletable=false)
);
-> at com.duckduckgo.app.browser.BrowserTabViewModelTest$whenCloseCurrentTabSelectedThenTabDeletedFromRepository$1.invokeSuspend(BrowserTabViewModelTest.kt:1621)

@cmonfortep cmonfortep self-assigned this Jan 7, 2021
@tvalle
Copy link
Contributor Author

tvalle commented Jan 11, 2021

I reflected the tests to the match new behavior. If there's any issue left please let me know

Copy link
Contributor

@cmonfortep cmonfortep left a comment

Choose a reason for hiding this comment

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

Thanks, great work. I've tested this and works as expected. 👍

@cmonfortep cmonfortep merged commit 51d6224 into duckduckgo:develop Jan 14, 2021
cmonfortep added a commit that referenced this pull request Jan 26, 2021
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