Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Refactor clearBrowsingDataPanel.js with commonForm #8194

Merged
merged 1 commit into from
Apr 22, 2017
Merged

Refactor clearBrowsingDataPanel.js with commonForm #8194

merged 1 commit into from
Apr 22, 2017

Conversation

luixxiul
Copy link
Contributor

@luixxiul luixxiul commented Apr 10, 2017

Closes #8192
Addresses #7989

  • Added CommonFormSmall
  • Added data-test-id and testIds
  • Removed clearBrowsingDataPanel from forms.less
  • Updated test code

Auditors:

Test Plan:

  1. Open Clear browsing data panel from the menu
  2. Test the cancel button works
  3. Reopen the panel
  4. Test the clear button works

screenshot 2017-04-11 1 04 19

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Test Plan:

@luixxiul
Copy link
Contributor Author

Note: most of camel cases should be fixed after #8136 is merged.

@luixxiul
Copy link
Contributor Author

@cezaraugusto Updated.

Copy link
Contributor

@cezaraugusto cezaraugusto left a comment

Choose a reason for hiding this comment

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

everything works ok but I got some failing tests after this refactoring. I think some selectors are missing and just by searching who they are and re-adding them as data-test-id as you did with other selectors would fix it.

for reference:

npm run test -- --grep="Clear Browsing Panel"

lmk if you need a hand with that

Closes #8192
Addresses #7989

- Added CommonFormSmall to commonForm.js
- Added data-test-id and testIds
- Removed clearBrowsingDataPanel from forms.less
- Updated test code

Auditors:

Test Plan:
1. Open Clear browsing data panel from the menu
2. Test the cancel button works
3. Reopen the panel
4. Test the clear button works
@luixxiul
Copy link
Contributor Author

@luixxiul
Copy link
Contributor Author

However on my local machine I got these errors:

  Clear Browsing Panel
    with saved state values
      1) saves the history switch state
    with history
      2) "before each" hook for "shows clearing options"
    with closedFrames
      3) "before each" hook for "clears closedFrames"
    with site settings
      4) "before each" hook for "clears site settings and permissions"


  0 passing (1m)
  4 failing

  1) Clear Browsing Panel with saved state values saves the history switch state:
     WaitUntilTimeoutError: element ([data-test-id="clearBrowsingDataButton"]) still not visible after 10000ms
      at isVisible("[data-test-id="clearBrowsingDataButton"]") - waitForVisible.js:37:22

  2) Clear Browsing Panel with history "before each" hook for "shows clearing options":
     Promise was rejected with the following reason: Error: unknown error: Cannot read property 'appStoreRenderer' of undefined
  Error

  3) Clear Browsing Panel with closedFrames "before each" hook for "clears closedFrames":
     Promise was rejected with the following reason: Error: unknown error: Cannot read property 'appStoreRenderer' of undefined
  Error

  4) Clear Browsing Panel with site settings "before each" hook for "clears site settings and permissions":
     Promise was rejected with the following reason: Error: unknown error: Cannot read property 'appStoreRenderer' of undefined
  Error

I think something is needed to complete the tests successfully, yet I'm not sure what it is.

@NejcZdovc
Copy link
Contributor

@luixxiul I run this test locally, it works on refactoring-aphrodite on this branch I get this errors:

  Clear Browsing Panel
    with saved state values
      1) saves the history switch state
    with history
      2) shows clearing options
      3) clears the browsing history
    with closedFrames
      ✓ clears closedFrames (350ms)
    with site settings
      4) clears site settings and permissions


  1 passing (1m)
  4 failing

  1) Clear Browsing Panel with saved state values saves the history switch state:
     WaitUntilTimeoutError: element ([data-l10n-id="security"]) still not visible after 10000ms
      at isVisible("[data-l10n-id="security"]") - waitForVisible.js:37:22

  2) Clear Browsing Panel with history shows clearing options:
     WaitUntilTimeoutError: element ([data-l10n-id="security"]) still not visible after 10000ms
      at isVisible("[data-l10n-id="security"]") - waitForVisible.js:37:22

  3) Clear Browsing Panel with history clears the browsing history:
     WaitUntilTimeoutError: element ([data-l10n-id="security"]) still not visible after 10000ms
      at isVisible("[data-l10n-id="security"]") - waitForVisible.js:37:22

  4) Clear Browsing Panel with site settings clears site settings and permissions:
     WaitUntilTimeoutError: element ([data-l10n-id="security"]) still not visible after 10000ms
      at isVisible("[data-l10n-id="security"]") - waitForVisible.js:37:22

@luixxiul
Copy link
Contributor Author

@NejcZdovc do you have any clues? I don't.

@NejcZdovc
Copy link
Contributor

@luixxiul I rerun it again and now it's all green. Not sure what was wrong in the first run.

Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

Run test manually and they pass, so LGTM

@luixxiul luixxiul merged commit db606f6 into brave:refactoring-aphrodite Apr 22, 2017
@luixxiul luixxiul deleted the clearBrowsingDataPanel-refactoring branch April 25, 2017 03:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants