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

[DIT-6187] Test coverage improvements #103

Merged
merged 7 commits into from
Mar 7, 2024

Conversation

azjgard
Copy link
Collaborator

@azjgard azjgard commented Mar 7, 2024

Overview

Improves mocking capabilities generally and adds a bunch of tests to improve coverage.

  • test: full coverage for determineModuleType
  • chore: add jest globals dep for typesafe mocking
  • test: substantial coverage improvements for token.ts + some pull.ts utils
  • test: add basic http tests
  • test: add mocks for http files, clean up http mocking in pull.test.ts
  • test: basic e2e for pull command

Context

https://linear.app/dittowords/issue/DIT-6187/cli-audit-existing-tests
https://linear.app/dittowords/issue/DIT-6186/cli-pull-integration-tests-which-validate-files-written-to-disk

Test Plan

  • Tests pass
  • Running commands unauthenticated correctly prompts for and saves the provided API token
  • pull writes files to disk with a non-default configuration

@azjgard azjgard requested a review from jaerod95 March 7, 2024 00:57
TEXT_DIR,
TEXT_FILE,
};
export default new (class {
Copy link
Collaborator Author

@azjgard azjgard Mar 7, 2024

Choose a reason for hiding this comment

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

In all places but one we already imported the default export of this file (and I refactored that one place), so this doesn't change anything functionally, but we can utilize the ES6 getter to dynamically set these values in tests by changing environment variables at runtime.

lib/init/token.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,367 @@
import fs from "fs";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This file was renamed from pull.test.ts to pull-lib.test.ts -- most of code shown as changed I didn't actually touch. I will leave a comment on the things that were changed.

});
});

const { getJsonFormat } = _test;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added tests from here down

lib/pull.test.ts Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be reviewed as a brand new file. All of the old code in the file was moved to pull-lib.test.ts.

Copy link
Contributor

@jaerod95 jaerod95 left a comment

Choose a reason for hiding this comment

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

Tested cli locally, everything is working great

@azjgard azjgard merged commit 68888a6 into master Mar 7, 2024
1 check passed
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