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

Convert suite settings (coins, custom-firmware) + refactors desktop pipeline #15869

Merged

Conversation

Vere-Grey
Copy link
Contributor

Converts:
settings/coins.test.ts
settings/custom-firmware.test.ts

Fixes:
settings/t2b1-device-settings.test.ts

Refactors and updates desktop pipeline. Works with tags and is updated according to the web one,

minor refactoring
some verification commented out and will be part of future refactoring
adds logic to use correct API url based on web/desktop/local/CI
removes flakyness of Coins test
better matrix
move docker pull from tests step to improve readability
pull only what is need
Attach playwright report for devs who dont have currents
add --forbid-only to both pipelines
GITHUB_ACTION: true
CURRENTS_PROJECT_ID: Og0NOQ
CURRENTS_RECORD_KEY: ${{ secrets.CURRENTS_RECORD_KEY }}
CURRENTS_CI_BUILD_ID: pr-run-${{github.run_id}}
run: |
docker compose up -d ${{ matrix.CONTAINERS }}
echo "Starting Playwright Web test group ${TEST_GROUP}"
yarn workspace @trezor/suite-desktop-core test:e2e:web --grep=${TEST_GROUP}
echo "Starting Playwright Web test group ${{ matrix.TEST_GROUP }}"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unify how vars are passed to bash and adds safety net --forbid-only. Playwright docs: Whether to disallow test.only. Useful on CI.

# - TEST_GROUP: "@group=other"
# CONTAINERS: "trezor-user-env-unix"
- TEST_GROUP: "@group=wallet"
CONTAINERS: "trezor-user-env-unix bitcoin-regtest"

steps:
- name: Checkout
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Our web pipeline uses different checkout:

      - name: Checkout
        uses: actions/checkout@v4
        with:
          ref: ${{github.event.after}}
          fetch-depth: 2

Any preferences?

@@ -34,7 +36,7 @@ const config: PlaywrightTestConfig = {
reporter: process.env.GITHUB_ACTION
? [['list'], ['@currents/playwright'], ['html', { open: 'never' }]]
: [['list'], ['html', { open: 'never' }]],
timeout: 1000 * 60 * 5,
timeout: process.env.GITHUB_ACTION ? timeoutCIRun : timeoutLocalRun,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want locally tests to fail fast during troubleshooting.

Comment on lines +106 to +115
export const getApiUrl = (webBaseUrl: string | undefined, testInfo: TestInfo) => {
const electronApiURL = 'file:///';
const apiURL =
testInfo.project.name === PlaywrightProjects.Desktop ? electronApiURL : webBaseUrl;
if (!apiURL) {
throw new Error('apiURL is not defined');
}

return apiURL;
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

might need update once i see first ci run

Comment on lines +21 to +26
await test.step('Select the custom firmware', async () => {
const fileChooserPromise = page.waitForEvent('filechooser');
await page.getByTestId('@firmware-modal/input-area').click();
const fileChooser = await fileChooserPromise;
await fileChooser.setFiles(firmwarePath);
});
Copy link
Contributor Author

@Vere-Grey Vere-Grey Dec 10, 2024

Choose a reason for hiding this comment

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

I am really impressed this works in desktop. Well done playwright

@HajekOndrej HajekOndrej merged commit 7fd04be into develop Dec 10, 2024
25 checks passed
@HajekOndrej HajekOndrej deleted the feat-convert-cy-pw-suite-settings-custom-firmware branch December 10, 2024 13:46
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