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

Migrate particle-cli to CircleCI #619

Merged

Conversation

DebbieGillespie
Copy link
Contributor

@DebbieGillespie DebbieGillespie commented Apr 27, 2022

Story details: https://app.shortcut.com/particle/story/102023

Migrate CI from Travis to CircleCI.

Notable changes/notes

  • Drops support for testing Node 8 and Node latest
  • Adds support for testing Node 14 and 16
  • Adds support for testing on Windows
  • Coverage report is accessible from the Artifacts tab of the run of the test-coverage job.
  • Drops notifications to slack channel
  • e2e tests run in sequence so as to ensure we do not encounter conflicts as they cannot run in parallel
  • Drops support for LibraryAddCommand as it was intermittently timing out and causing tests to fail
  • Not clear how to test the npm publish given this is a public repo

@DebbieGillespie
Copy link
Contributor Author

@busticated I have the unit tests running for multiple versions of node on both MacOS and Linux. The node 8 tests are failing due to the following error:

Requires optional dependency:
     Error: Cannot find module 'usb'

Before I dig into this, want to confirm that we still want to test on node 8. If yes, any suggestions on how to get the usb dependency to installed for node8?

@DebbieGillespie DebbieGillespie force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from 6a799b1 to d0629dc Compare April 27, 2022 18:02
@DebbieGillespie DebbieGillespie force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from 7b49b67 to 0c16c50 Compare April 27, 2022 22:18
@DebbieGillespie DebbieGillespie force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from 7101cf6 to 4a8c8af Compare April 27, 2022 23:23
@monkbroc monkbroc force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch 3 times, most recently from fdccc81 to 0318af1 Compare April 29, 2022 14:53
@monkbroc monkbroc force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from 731635a to 6fff1bc Compare April 29, 2022 15:03
@monkbroc
Copy link
Member

I took a stab at running the tests on Windows as well. I skipped running the e2e tests since the failures there were harder to debug.

The main source of failures I'm seeing is that we're now running several e2e tests in parallel and they sometimes conflict with each other, for example removing a device from the product in one job but expecting it to be in the product in another job. It might be best to run e2e tests in sequence but then the tests will be super slow.

@monkbroc
Copy link
Member

This unit test frequently fails so I'd advocate removing it

  1) LibraryAddCommand
       adds a library to a project:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/distiller/project/src/cli/library_add.test.js)
      at listOnTimeout (node:internal/timers:559:17)
      at processTimers (node:internal/timers:502:7)

@monkbroc monkbroc force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from 004356f to de6b48b Compare May 2, 2022 20:14
@monkbroc monkbroc force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from de6b48b to 7c9dda9 Compare May 2, 2022 20:16
Checking to see if those are still running successfully.
This test often encounters a timeout before completing. Commenting out
until we have time to address why the test is taking so long.
@DebbieGillespie DebbieGillespie force-pushed the feature/sc-102023/migrate-particle-cli-to-circleci branch from 5b3e1c0 to ce8f5bd Compare May 3, 2022 15:14
@monkbroc
Copy link
Member

monkbroc commented May 3, 2022

I'm glad to see all the ✔️ on the CircleCI checks! Thanks for the work here, Debbie! Mirande, I put the E2E tests in a sequence to avoid false failures when tests for the same feature ran in parallel.

@DebbieGillespie DebbieGillespie marked this pull request as ready for review May 3, 2022 23:37
Copy link
Member

@monkbroc monkbroc left a comment

Choose a reason for hiding this comment

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

Looks great to me. Mirande, does the CI setup meet your expectations?

Copy link
Contributor

@busticated busticated left a comment

Choose a reason for hiding this comment

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

looks good - thanks y'all 🙏 👍

@DebbieGillespie DebbieGillespie merged commit 555debd into master May 5, 2022
@DebbieGillespie DebbieGillespie deleted the feature/sc-102023/migrate-particle-cli-to-circleci branch May 5, 2022 00:13
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.

3 participants