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

chore: Testing #1399

Merged
merged 13 commits into from
Aug 27, 2021
Merged

chore: Testing #1399

merged 13 commits into from
Aug 27, 2021

Conversation

maxwellmattryan
Copy link
Contributor

@maxwellmattryan maxwellmattryan commented Aug 13, 2021

Description of change

This PR integrates Jest for testing the Typescript files in packages/shared/lib, which contains mostly business logic for all platforms of Firefly. This will be important for whenever we would like to add new functionality, refactor existing functionality, etc.

I tried integrating Mocha as well, but was not able to successfully setup a mocked DOM environment for testing (despite trying jsdom). For this reason, Jest was chosen as it ships with jsdom making it quite easy to setup.

Changes

  • Adds firefly-ci.yml for continuous integration checks (linting will be added later)
  • New directory for tests (shared/lib/test) along with sample test file currency.test.ts
    • It's likely that scenarios we add tests for later will require some extra configuration and setup - use shared/lib/test/setup.js

Links to any relevant issues

None

Type of change

  • Chore (refactoring, build scripts or anything else that isn't user-facing)

How the change has been tested

Tested on:

  • MacOS (Catalina 10.15.7)
  • Ubuntu (20.04)

Change checklist

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@maxwellmattryan maxwellmattryan added the type:chore House-keeping etc. label Aug 13, 2021
@@ -104,7 +104,7 @@ export const activeProfile: Readable<Profile | undefined> = derived(
)

activeProfileId.subscribe((profileId) => {
Electron.updateActiveProfile(profileId)
Electron?.updateActiveProfile(profileId)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change exists because it was throwing errors of reading updateActiveProfile of undefined.

.github/workflows/pr-checks.yml Outdated Show resolved Hide resolved
.github/workflows/pr-checks.yml Outdated Show resolved Hide resolved
@maxwellmattryan maxwellmattryan marked this pull request as ready for review August 25, 2021 21:07
Copy link
Member

@rajivshah3 rajivshah3 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! Just two minor things

.github/workflows/firefly-ci.yml Outdated Show resolved Hide resolved
.github/workflows/firefly-ci.yml Outdated Show resolved Hide resolved
Copy link
Member

@rajivshah3 rajivshah3 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!

@maxwellmattryan maxwellmattryan merged commit 137be9a into develop Aug 27, 2021
@maxwellmattryan maxwellmattryan deleted the chore/testing branch August 27, 2021 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:ci Affects CI actions type:chore House-keeping etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants