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 jest config #6

Merged
merged 2 commits into from
Oct 3, 2022
Merged

Add jest config #6

merged 2 commits into from
Oct 3, 2022

Conversation

JuanVqz
Copy link
Member

@JuanVqz JuanVqz commented Oct 2, 2022

  • Add jest config
  • Test a basic existing function.

Interesting Notes:
How to configure jest with typescript? https://jestjs.io/docs/getting-started
Reference to globals in jest? https://jestjs.io/docs/api
How to mock in jest? https://jestjs.io/docs/mock-function-api#typescript-usage

depngn 12.0.0 --reporter=json
`

console.log(usage);
Copy link
Member Author

@JuanVqz JuanVqz Oct 2, 2022

Choose a reason for hiding this comment

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

As you can see in the image the message now has a kind of left-padding.
if you disagree with that I can revert the change, also, If you prefer using multi-console logs I can revert that as well.

lll-2022-10-02_00-19

Copy link
Contributor

Choose a reason for hiding this comment

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

oohh, i actually kind of like this!

Copy link
Contributor

Choose a reason for hiding this comment

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

the only weird thing that might pop up is if we set up automatic formatting/linting -- it might mistakenly try to "fix" this and add even more padding. we can cross that bridge when we come to it though.

createUsage();

const usage = `
Usage:
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't have the wrong indentation, it must be like that in order to match what createUsage() returns.

@@ -23,3 +23,6 @@ dist-ssr
*.njsproj
*.sln
*.sw?

# testing
coverage/
Copy link
Member Author

Choose a reason for hiding this comment

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

we will know the project's coverage.

image

@JuanVqz JuanVqz changed the title Add jest with example test Add jest config Oct 2, 2022
Copy link
Contributor

@kindoflew kindoflew left a comment

Choose a reason for hiding this comment

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

Awesome, thanks!! I just had a couple suggestions, but it mostly LGTM!

I actually started work on this on Friday, but I did mostly the same stuff as you (with the addition of my comments/requests).

jest.config.ts Outdated
collectCoverage: true,

// An array of glob patterns indicating a set of files for which coverage information should be collected
// collectCoverageFrom: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we specify which files to collect from and which ones to ignore? something like:

Suggested change
// collectCoverageFrom: undefined,
collectCoverageFrom: [
'src/**/*.ts',
'!src/**/index.ts',
'!src/**/types.ts'
],

@@ -0,0 +1,28 @@
import { describe, expect, it, jest } from '@jest/globals';
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use the globals that don't have to be imported? i know "magic" values aren't always the best, but it seems to be the common convention when using jest.

I think you can npm install -D @types/jest and then add "jest" to the types array in tsconfig.json.

depngn 12.0.0 --reporter=json
`

console.log(usage);
Copy link
Contributor

Choose a reason for hiding this comment

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

oohh, i actually kind of like this!

depngn 12.0.0 --reporter=json
`

console.log(usage);
Copy link
Contributor

Choose a reason for hiding this comment

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

the only weird thing that might pop up is if we set up automatic formatting/linting -- it might mistakenly try to "fix" this and add even more padding. we can cross that bridge when we come to it though.

@JuanVqz
Copy link
Member Author

JuanVqz commented Oct 3, 2022

comments addressed, @kindoflew!

@JuanVqz JuanVqz requested a review from kindoflew October 3, 2022 02:51
@kindoflew kindoflew merged commit c931d68 into upgradejs:main Oct 3, 2022
@JuanVqz JuanVqz deleted the add-jest branch October 3, 2022 13:57
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