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

Added verdaccio test #268

Merged
merged 7 commits into from
May 7, 2023
Merged

Conversation

rluvaton
Copy link
Collaborator

No description provided.

@request-info
Copy link

request-info bot commented Apr 28, 2023

Thanks for opening this PR! We would appreciate it if you could provide us with more info about your PR 🙏

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

❗ No coverage uploaded for pull request base (blog/crucial-tests@06f7655). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@                  Coverage Diff                  @@
##             blog/crucial-tests     #268   +/-   ##
=====================================================
  Coverage                      ?   90.92%           
=====================================================
  Files                         ?       16           
  Lines                         ?      595           
  Branches                      ?       43           
=====================================================
  Hits                          ?      541           
  Misses                        ?       53           
  Partials                      ?        1           
Flag Coverage Δ
app 91.97% <0.00%> (?)
generator 63.63% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rluvaton rluvaton changed the title Update index.md Added verdaccio test Apr 30, 2023
@rluvaton
Copy link
Collaborator Author

I feel like it needs some more content... 😕

Copy link
Contributor

@goldbergyoni goldbergyoni left a comment

Choose a reason for hiding this comment

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

@rluvaton The code example is awesome, let's spend some short time doing justice with it and wrapping it with great texts and examples?

@@ -394,6 +392,50 @@ beforeAll(() => {
});
```

## 📦 Test the package as a consumer

a lot of times we don't test our packages as how our consumers will use them,
Copy link
Contributor

Choose a reason for hiding this comment

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

Not the same format like other bullets, e.g., "What & why:" section

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, fixed

## 📦 Test the package as a consumer

a lot of times we don't test our packages as how our consumers will use them,
this is a simplified example of how we could do that
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing here a simple explanation of a problem, e.g. '100% of your library's tests pass, the tests are great, they cover all the scenarios, but shockingly they fail in production, how come?? while the tests work against the local developer files, your user will work against the artifacts that were packed and zipped. If a single file is excluded due to build configuration or becauase of .npmignore, the code will fail...'

I can write myself, but I find it to be a great exercise for Raz the super-technical engineer to explain things in a CTO-level language:)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks, updated

this is a simplified example of how we could do that

```js
// global-setup.js
Copy link
Contributor

@goldbergyoni goldbergyoni May 1, 2023

Choose a reason for hiding this comment

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

I wouldn't couple our article to Jest, the before hook is more universal

Copy link
Collaborator Author

@rluvaton rluvaton May 5, 2023

Choose a reason for hiding this comment

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

global setup is not coupled to jest, I used global setup to show that this should be run once in all tests - hence global (also the before run in each file)

await setupVerdaccio();

// 2. Building our package
await execa('npm', ['run', 'build'], {
Copy link
Contributor

@goldbergyoni goldbergyoni May 1, 2023

Choose a reason for hiding this comment

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

execa is unneccessary concept that the reader should reason about

exec('command') is almost as easy and standard, or at least put a comment explaining that execa runs chid-process commands

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

changed to exec


a lot of times we don't test our packages as how our consumers will use them,
this is a simplified example of how we could do that

Copy link
Contributor

Choose a reason for hiding this comment

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

Before the working example, I would consider showing the failing example briefly:

// package.json, only the files entry with index.js only

// index.js file which require 'lib-internal.js file

// local test that require lib-internal/function, explain with comments that it will work on developer machine only

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

created a failing example while guiding the reader to the trap (the mistake that happen)

@goldbergyoni goldbergyoni merged commit 46f4b4e into blog/crucial-tests May 7, 2023
@goldbergyoni goldbergyoni deleted the cruicial-tests-code-change branch May 7, 2023 15:00
@goldbergyoni
Copy link
Contributor

@rluvaton I merged first before reviewing as I prefer to review the entire article, all together, before merging to main

For now it seems 98% great, leme revise one time more before publishing

goldbergyoni added a commit that referenced this pull request Jul 5, 2023
* First draft

* First draft

* Added verdaccio test (#268)

* Update index.md

* fix comment

* added the verdaccio test

* Update index.md

* used import/export and update content

* update content

* update content

* Genric message

* Genric message

* Genric message

---------

Co-authored-by: Yoni Goldberg <goldbergyoni@Yonis-MacBook-Pro-2.local>
Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
goldbergyoni added a commit that referenced this pull request Jul 5, 2023
* First draft

* First draft

* Added verdaccio test (#268)

* Update index.md

* fix comment

* added the verdaccio test

* Update index.md

* used import/export and update content

* update content

* update content

* Genric message

* Genric message

* Genric message

* Genric message

* Genric message

---------

Co-authored-by: Yoni Goldberg <goldbergyoni@Yonis-MacBook-Pro-2.local>
Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
goldbergyoni added a commit that referenced this pull request Jul 7, 2023
* First draft

* First draft

* Added verdaccio test (#268)

* Update index.md

* fix comment

* added the verdaccio test

* Update index.md

* used import/export and update content

* update content

* update content

* Genric message

* Genric message

* Genric message

* Genric message

* Genric message

* Correct date

---------

Co-authored-by: Yoni Goldberg <goldbergyoni@Yonis-MacBook-Pro-2.local>
Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
goldbergyoni added a commit that referenced this pull request Jul 7, 2023
* First draft

* First draft

* Added verdaccio test (#268)

* Update index.md

* fix comment

* added the verdaccio test

* Update index.md

* used import/export and update content

* update content

* update content

* Genric message

* Genric message

* Genric message

* Genric message

* Genric message

* Correct date

* Correct date

---------

Co-authored-by: Yoni Goldberg <goldbergyoni@Yonis-MacBook-Pro-2.local>
Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
goldbergyoni added a commit that referenced this pull request Jul 11, 2023
* First draft

* First draft

* Added verdaccio test (#268)

* Update index.md

* fix comment

* added the verdaccio test

* Update index.md

* used import/export and update content

* update content

* update content

* Genric message

* Genric message

* Genric message

* Genric message

* Genric message

* Correct date

* Correct date

* Genric message2

* Genric message2

---------

Co-authored-by: Yoni Goldberg <goldbergyoni@Yonis-MacBook-Pro-2.local>
Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
goldbergyoni added a commit that referenced this pull request Jul 11, 2023
* First draft

* First draft

* Added verdaccio test (#268)

* Update index.md

* fix comment

* added the verdaccio test

* Update index.md

* used import/export and update content

* update content

* update content

* Genric message

* Genric message

* Genric message

* Genric message

* Genric message

* Correct date

* Correct date

* Genric message2

* Genric message2

* Genric message2

---------

Co-authored-by: Yoni Goldberg <goldbergyoni@Yonis-MacBook-Pro-2.local>
Co-authored-by: Raz Luvaton <16746759+rluvaton@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants