Skip to content

Conversation

ruhankiani
Copy link
Contributor

Relevant Links

Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1987735
TestRail: 246978

Description of Code / Doc Changes

  • Created a test file to verify multiple selected tabs can be pinned and unpinned
  • Create a method for TabBar in modules/browser_object_tabbar.py for executing the action chain of selecting multiple tabs based on a list of indices.

Process Changes Required

Mark the relevant boxes:

  • Adds a dependency (rerun pipenv install)
  • [] Changes the BasePage
  • Changes or creates a BOM/POM (name the object model): _
  • Changes CI flow
  • Changes scheduled Beta or DevEdition
  • Changes Git hooks or Github settings
  • Changes L10n harness

Screenshots or Explanations

If you need to explain your code, do it here.

Comments or Future Work

Do we need to start another PR soon to address something you saw while working on this?

Workflow Checklist

  • Please request reviewers
  • If this is an unblocker, please post in Slack.
  • If asked to address comments, please resolve conversations.
  • If asked to change code, please re-request review from the person who wanted changes.

Thank you!

Copy link
Collaborator

@Tracy-Walker Tracy-Walker left a comment

Choose a reason for hiding this comment

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

pull --rebase origin main (that will get rid of the form_autofiull test failures in CI check)
Remove the test inline use of with driver.context(driver.CONTEXT_CHROME):
commit and push and you should be good to go.
Nice work on the multi-select tabs method, In fact, please rename it to be even clearer.. maybe select_multiple_tabs_by_indices(

driver.get(URLS[i])
assert len(driver.window_handles) == 5

with driver.context(driver.CONTEXT_CHROME):
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not necessary to use
with driver.context(driver.CONTEXT_CHROME):
All of the methods called already declare it, like in the method you created.

@ruhankiani
Copy link
Contributor Author

I made the requested changes and rebased but it's still failing the CI smoke tests, not sure why.

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