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

Migration Challenge to GitHub Actions #1707

Closed
wants to merge 3 commits into from

Conversation

serima
Copy link
Contributor

@serima serima commented Apr 27, 2023

We have created a pull request for GitHub Actions migration to resolve this issue.
Tests that depend on BrowserStack (e.g. pnpm run test:unit) were commented out because they always fail, but we temporarily uncommented them all.

It seems that I need the approval of the maintainer to enable Workflow in the Dexie.js repository, so can you try and merge it once approved?
Also, I would like you to try setting up BrowserStack.
https://www.browserstack.com/docs/automate/selenium/github-actions

We are thinking that it would be good to run it in parallel with TravisCI until we can confirm that it is stable and working.

@serima serima changed the title [WIP] Migration Challenge to GitHub Actions Migration Challenge to GitHub Actions Apr 27, 2023
@serima
Copy link
Contributor Author

serima commented Apr 27, 2023

@dfahlander Could you check this pull request?

@dfahlander
Copy link
Collaborator

Thanks a lot! If we can run the tests on Github Actions with local testing only (not checking for browserstack env vars), but run it against a local browser on the github actions host if possible, that would be great. Regarding Browserstack, we are moving over to Lambdatest instead of browserstack (which is in another PR #1701). If we can fix this PR with local testing, we could also adjust #1701 to use GA if needed.

@serima
Copy link
Contributor Author

serima commented Apr 28, 2023

@dfahlander Thanks for the quick reply!

If we can run the tests on Github Actions with local testing only (not checking for browserstack env vars)
I also think this approach is nice.

So, for #1707, let's use GitHub Actions, with the goal of passing the tests on local testing only first, shall we?
Are the tests that depend on BrowserStack isolated on a per-execution-command basis?

I looked at this a bit yesterday,
It seemed to me that the tests run through karma with pnpm run test:unit are dependent on BrowserStack.

@dfahlander
Copy link
Collaborator

Browserstack only optionally used based on env variable check in karma.common.js when assigning browserSuiteToUse

const browserSuiteToUse = process.env.NODE_ENV === 'release' ?
  "pre_npm_publish" :
  process.env.TRAVIS ?
    isNaN(process.env.TRAVIS_PULL_REQUEST) && process.env.BROWSER_STACK_USERNAME ?
      "ci" :              // CI pushs to master and browserstack credentials exists
      "ciLocal" :         // CI pull request or has no browserstack credentials.
  "local";                // Developer local machine

The "local" or "ciLocal" variants can be used. The only difference between them is that "local" is using Chrome (because it's installed on most dev machines) and "ciLocal" uses a local Firefox which installed in Travis using . addons: firefox: "85.0" in .travis.yml.

If github actions also has the possibility to install a browser (or if it's already part of the environment), then we could use that one as the ciLocal option.

The browser-set to use depending on "local", "ciLocal", etc are defined in karma.browsers.matrix.js

/** This module comprises the list of browsers
 * to run tests on depending on environment.
 *
 * Browsers listed here must also be defined in
 * karma.browserstack.js
 */

module.exports = {
    // On developers machines, Chrome is most likely to be installed.
    local: ['Chrome'],
    //local: ['bs_safari_latest_supported'],

    // When browserstack cannot be used, use local Firefox.
    ciLocal: ['Firefox'],
    
    // Continous Integration on every push to master
    ci: [
        // - Let firefox represent the standard evergreen browser.
        // Leaving out Chrome, since local tests have hopefully already run on it.
        // Chrome will be tested in the pre_npm_publish anyway.
        'bs_firefox_latest_supported', 
        // Safari. Enforces native Safari support for every PR!
        'bs_safari_latest_supported'
    ],

    // Test matrix used before every npm publish.
    pre_npm_publish: [
        'bs_chrome_oldest_supported', // ...because not tested in CI!
        'bs_chrome_latest_supported', // ...because not tested in CI!
        'bs_firefox_oldest_supported', // ...because not tested in CI!
        "bs_safari_oldest_supported", // ...because not tested in CI!
    ]
}

@dfahlander
Copy link
Collaborator

Thanks for this PR! I could use it with some modifications to get everything working. Replaced with PRs #1747, #1748 and #1749 #1750 .

@dfahlander dfahlander closed this Jun 18, 2023
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.

Migrate to Github Actions
2 participants