Skip to content
This repository has been archived by the owner on Jun 17, 2022. It is now read-only.

Move tests to correct sub-package #576

Closed
wants to merge 14 commits into from
Closed

Move tests to correct sub-package #576

wants to merge 14 commits into from

Conversation

Hinton
Copy link
Member

@Hinton Hinton commented Dec 10, 2021

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

We previously split up the jslib repo into multiple modules but the tests remained in the root project. With npm v7 having support for workspaces, I sent some time cleaning it up and moving the tests to the correct packages.

This should also allow us to properly build each package independently and use the built files vs depending on the typescript source. Which I believe will help alleviate many of the issues we've encountered.

Before you submit

  • I have checked for linting errors (npm run lint) (required)
  • I have added unit tests where it makes sense to do so (encouraged but not required)
  • This change requires a documentation update (notify the documentation team)
  • This change has particular deployment requirements (notify the DevOps team)

@Hinton Hinton marked this pull request as ready for review December 10, 2021 15:17
common/karma.conf.js Outdated Show resolved Hide resolved
@Hinton
Copy link
Member Author

Hinton commented Dec 15, 2021

Tests fails due on macos due to karma-webpack bug. Fixed in master but no new release yet. Pondering if we can tie it to a specific commit for now or hold off until it's merged.

…workspaces

# Conflicts:
#	common/spec/importers/fsecureFskImporter.spec.ts
#	common/spec/services/cipher.service.spec.ts
#	package-lock.json
#	package.json
…workspaces

# Conflicts:
#	spec/common/importers/firefoxCsvImporter.spec.ts
#	spec/common/importers/fsecureFskImporter.spec.ts
#	spec/common/importers/lastpassCsvImporter.spec.ts
#	spec/common/importers/nordpassCsvImporter.spec.ts
#	spec/common/importers/onepassword1PifImporter.spec.ts
#	spec/common/importers/onepasswordMacCsvImporter.spec.ts
#	spec/common/importers/onepasswordWinCsvImporter.spec.ts
#	spec/common/misc/sequentialize.spec.ts
#	spec/common/misc/throttle.spec.ts
#	spec/common/misc/utils.spec.ts
#	spec/common/services/cipher.service.spec.ts
#	spec/common/services/consoleLog.service.spec.ts
#	spec/common/services/export.service.spec.ts
#	spec/electron/services/electronLog.service.spec.ts
#	spec/electron/utils.spec.ts
#	spec/node/cli/consoleLog.service.spec.ts
#	spec/node/services/nodeCryptoFunction.service.spec.ts
#	spec/support/jasmine.json
#	spec/support/karma.conf.js
#	spec/utils.ts
#	spec/web/services/webCryptoFunction.service.spec.ts
@Hinton Hinton added the hold Do not merge, do not approve yet label Jan 3, 2022
@MGibson1
Copy link
Member

MGibson1 commented Jan 27, 2022

Tests fails due on macos due to karma-webpack bug. Fixed in master but no new release yet. Pondering if we can tie it to a specific commit for now or hold off until it's merged.

Do you happen to have an issue/commit to link to?

@Hinton
Copy link
Member Author

Hinton commented Jan 28, 2022

@MGibson1
Copy link
Member

MGibson1 commented Apr 4, 2022

This was done in #744

@MGibson1 MGibson1 closed this Apr 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
hold Do not merge, do not approve yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants