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

fix: for starting accounts at Account 1 and added new test #1281

Merged
merged 7 commits into from
May 27, 2024

Conversation

HoTandy
Copy link
Collaborator

@HoTandy HoTandy commented May 7, 2024

  • Using the mock object, the Accounts were labelled starting at Account 0, this doesn't match reality so now they start at Account 1

  • In popup.ts, I've added functions for doing the following through the UI
    -- creating an account - createAccount
    -- creating an account from private key - createAccountFromPrivateKey
    -- extracting text from the clipboard - getClipboardText
    -- getting the fuel address for an account number - getAddressForAccountNumber
    -- exporting the private key - exportPrivateKey (not yet used and not sure if it's working properly)

In Accounts.test.ts I've created a test that checks to make sure the same addresses are getting added when we add new accounts, even after importing an account using that account's private key.

Copy link
Contributor

github-actions bot commented May 21, 2024

Coverage report for ./packages/app

St.
Category Percentage Covered / Total
🟡 Statements 62.51% 3228/5164
🔴 Branches 42.05% 627/1491
🔴 Functions 46.94% 682/1453
🟡 Lines 63.07% 3110/4931

Test suite run success

240 tests passing in 70 suites.

Report generated by 🧪jest coverage report action from ffc44ac

Copy link
Member

@helciofranco helciofranco left a comment

Choose a reason for hiding this comment

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

Just two comments:

  1. Missing to add a changeset for these changes.
    It can be an "empty" changeset since these changes are not in the app itself.
    pnpm changeset --empty and then commit and push that file.
Screenshot 2024-05-24 at 12 09 23
  1. Also, the PR title should be updated to use the prefix "fix:" (there's a CI routine failing due to the PR title).
Screenshot 2024-05-24 at 12 09 36

@HoTandy HoTandy changed the title Fix for starting accounts at Account 1 and added new test fix: for starting accounts at Account 1 and added new test May 27, 2024
Copy link
Contributor

@LuizAsFight LuizAsFight left a comment

Choose a reason for hiding this comment

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

why pnpm-lock changed? I dont see any changes in dependencies

@LuizAsFight LuizAsFight merged commit 9e8e0fb into master May 27, 2024
12 checks passed
@LuizAsFight LuizAsFight deleted the pw-e2e-new-test branch May 27, 2024 17:30
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.

None yet

3 participants