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

Add vitest and arrow eslint plugins #185

Merged
merged 7 commits into from
Jun 23, 2023
Merged

Add vitest and arrow eslint plugins #185

merged 7 commits into from
Jun 23, 2023

Conversation

danvk
Copy link
Collaborator

@danvk danvk commented Jun 22, 2023

This adds two new eslint plugins:

This was prompted by discovering two tests with the same name in #186.

@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #185 (9613e1d) into main (354a762) will decrease coverage by 0.23%.
The diff coverage is 95.23%.

@@            Coverage Diff             @@
##             main     #185      +/-   ##
==========================================
- Coverage   66.06%   65.83%   -0.23%     
==========================================
  Files          87       87              
  Lines        4370     4338      -32     
  Branches      323      322       -1     
==========================================
- Hits         2887     2856      -31     
+ Misses       1466     1465       -1     
  Partials       17       17              
Impacted Files Coverage Δ
src/App.tsx 0.00% <0.00%> (ø)
src/editor/TipTapEditor.tsx 0.00% <0.00%> (ø)
src/panels/references/ReferencesPanel.tsx 85.24% <50.00%> (ø)
...helpers/unsetPartiallySelectedCollapsibleBlocks.ts 94.87% <100.00%> (ø)
...apsibleBlock/keyboardShortcutCommands/backspace.ts 100.00% <100.00%> (ø)
...collapsibleBlock/keyboardShortcutCommands/enter.ts 100.00% <100.00%> (ø)
...lapsibleBlock/keyboardShortcutCommands/modEnter.ts 100.00% <100.00%> (ø)
.../collapsibleBlock/nodes/CollapsibleBlockContent.ts 100.00% <100.00%> (ø)
...des/collapsibleBlock/nodes/CollapsibleBlockNode.ts 94.18% <100.00%> (-0.25%) ⬇️
.../collapsibleBlock/nodes/CollapsibleBlockSummary.ts 100.00% <100.00%> (ø)
... and 8 more

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@danvk danvk marked this pull request as ready for review June 22, 2023 20:57
@@ -22,7 +22,7 @@ describe('chatWithAI', () => {
expect(response).toStrictEqual(['some reply']);
});

test('should return list of strings with content returned by sidecar', async () => {
it('should return list of strings with content returned by sidecar 2', async () => {
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 test had the same name as the previous one but different content.

@@ -47,7 +47,7 @@ function LeftSidePanelWrapper() {
const [primaryPane, setPrimaryPane] = useState<PrimarySideBarPane>('Explorer');
const openReference = useSetAtom(openReferenceAtom);

function handleSideBarClick(selectedPane: PrimarySideBarPane) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

function statements inside other functions are frowned upon because they're hoisted, just like variables declared with var.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What is wrong with that? Is there any other advantages of using function expression? (I know that is easier to define the function).

@@ -10,7 +10,7 @@ describe('chatWithAI', () => {
vi.clearAllMocks();
});

test('should return list of strings with content returned by sidecar', async () => {
it('should return list of strings with content returned by sidecar', async () => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The consistent-test-it rule says to use it inside describe blocks and test outside.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's ok. Thanks for the bulk rename :-).

@@ -19,19 +19,19 @@ describe('Chevron input rule handler', () => {
expect(collapsibleBlocks).toHaveLength(1);

const [collapsibleBlock] = collapsibleBlocks;
expect(getText(collapsibleBlock)).toEqual('Some content');
expect(getText(collapsibleBlock)).toBe('Some content');
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

},
];
},
parseHTML: () => [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't mind the left-hand side of this diff, but using arrow functions does allow you to drop the return for the common case of a one-liner.

});

test('should add a new content line to an uncollapsed collapsible block', () => {
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 was an exact duplicate of the previous test.

@danvk danvk requested a review from cguedes June 22, 2023 20:58
@cguedes cguedes merged commit 540c5cb into main Jun 23, 2023
10 of 11 checks passed
@cguedes cguedes deleted the lint-arrow branch June 23, 2023 09:07
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.

None yet

3 participants