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

feat: Moves the test utils generation script to this repo #77

Merged
merged 13 commits into from
Jan 15, 2025

Conversation

orangevolon
Copy link
Contributor

@orangevolon orangevolon commented Nov 28, 2024

Description of changes:

Centralizes the logic repeated in these four files:

  1. Components: test-utils.js
  2. Code view: test-utils.js
  3. Chat components: test-utils.js
  4. Board components: test-utils.js

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Issue #, if available:

Will be followed by PRs:

Related API Proposal: Test utils component finder generator

Copy link

codecov bot commented Nov 28, 2024

Codecov Report

Attention: Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.92%. Comparing base (75b167d) to head (f99a944).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/converter/generate-test-utils.ts 96.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #77      +/-   ##
==========================================
+ Coverage   95.75%   95.92%   +0.16%     
==========================================
  Files           6        9       +3     
  Lines         283      319      +36     
  Branches       65       68       +3     
==========================================
+ Hits          271      306      +35     
- Misses         12       13       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@orangevolon orangevolon force-pushed the chore/adds-test-utils-build-scripts branch from 98b36f6 to 8f3e6af Compare December 2, 2024 14:28
*/
export function getComponentMetadata(componentName: string) {
return ${buildComponentsMetadataMap(components)}[componentName];
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The above function is the only one that is new. Everything else is just transferred from other repos.

@orangevolon orangevolon force-pushed the chore/adds-test-utils-build-scripts branch from 7d6968b to aae87eb Compare January 13, 2025 14:37
@@ -27,13 +27,15 @@
"css-selector-tokenizer": "^0.8.0",
"css.escape": "^1.5.1",
"glob": "^7.2.0",
"lodash": "^4.17.21",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use lodash for casing conversion.

generateTestUtils({
components: mockComponents,
testUtilsPath,
});
Copy link
Contributor Author

@orangevolon orangevolon Jan 13, 2025

Choose a reason for hiding this comment

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

Since generateTestUtils is called inside the build phase (npm build) rather than the test runner (npm test), test runner doesn't include this inside the test coverage report.

For this reason we had to add additional unit tests to the individual functions to keep the test coverage high.

@@ -0,0 +1,53 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
Copy link
Contributor Author

@orangevolon orangevolon Jan 13, 2025

Choose a reason for hiding this comment

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

This is the second test file that we had to write to keep the test coverage high.

@@ -0,0 +1,74 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
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 is one of the unit tests that we had to write to keep the test coverage high.

@orangevolon orangevolon marked this pull request as ready for review January 13, 2025 16:00
@orangevolon orangevolon requested a review from a team as a code owner January 13, 2025 16:00
@orangevolon orangevolon requested review from georgylobko and removed request for a team January 13, 2025 16:00

const testUtilsFilePartialContent = 'ElementWrapper.prototype.findAlert';

expect(writeFileSync).toHaveBeenCalledWith(
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious how does it work? I believe the mocked version of this function should be used instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since mock calls (vi.mock) are hoisted to the top of the execution context, every function that is imported from that path is will be mocked. That's actually why this assertion is passing, otherwise we would get something like:

TypeError: [Function] is not a spy or a call to a spy!

georgylobko
georgylobko previously approved these changes Jan 15, 2025
@orangevolon orangevolon added this pull request to the merge queue Jan 15, 2025
Merged via the queue into main with commit 732469d Jan 15, 2025
36 checks passed
@orangevolon orangevolon deleted the chore/adds-test-utils-build-scripts branch January 15, 2025 15:08
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