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.

Sign up for GitHub

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

Jump to bottom

Ref: using node:assert/strict instead of if + throw #1371

Merged
merged 27 commits into from
Dec 11, 2023
Merged

Ref: using node:assert/strict instead of if + throw #1371

merged 27 commits into from
Dec 11, 2023

Conversation

RobinTail
Copy link
Owner

Reducing branch complexity that way.
⚠️ Requires to get rid of Jest: #1369

@RobinTail
Switching unit, system, integration and ESM tests to vitest.
2428aab
@RobinTail
Making compatibility test to test against jest.
b20c9dc
@RobinTail
Restoring jest as optional peer.
11b8375
@RobinTail
Fix compatibility test: each and dependencies.
8d5d99b
@RobinTail
Ref: combining imports in test.
2d09e45
@RobinTail
Minor: jsdoc.
3370420
@RobinTail
Deduplicating by upgrading all dependencies.
6b58682
@RobinTail
Ref: extracting makeLoggerMock() and reusing testing helpers in resul…
b38d9a1 
…t handler test instead of let.
@RobinTail
Ref: replacing let loggerMock in routing test.
cbe8719
@RobinTail
Ref: reusing appMock from express-mock in routing test.
0b98bd4
@RobinTail
Ref: replacing let in App system test, using the power of async descr…
150c39c 
…ibe (vitest feature).
@RobinTail
Ref: replacing let with await in async describe of ESM, integration a…
69603f3 
…nd 2 system tests, completely.
@RobinTail
Verbose reporter.
5ac2cd2
@RobinTail
Basic reporter - performance improvement to 10s.
8c10ad0
@RobinTail
Using non-isolated threads: 7.22s.
8dee8e0
@RobinTail
Revert "Using non-isolated threads: 7.22s."
778bb46 
This reverts commit 8dee8e0.
@RobinTail
Ref: Using node:assert/strict instead of if-throw. Reducing branch co…
0f28230 
…mplexity.
@RobinTail RobinTail added the refactoring The better way to achieve the same result label Dec 10, 2023
@RobinTail RobinTail marked this pull request as ready for review December 10, 2023 23:15
RobinTail
RobinTail commented Dec 11, 2023
View reviewed changes
RobinTail
RobinTail commented Dec 11, 2023
View reviewed changes
RobinTail
RobinTail commented Dec 11, 2023
View reviewed changes
RobinTail
RobinTail commented Dec 11, 2023
View reviewed changes
Base automatically changed from switch-to-vitest to master December 11, 2023 14:26
@coveralls
Copy link

coveralls commented Dec 11, 2023 •
edited
Loading

Coverage Status

coverage: 100.0%. remained the same
when pulling 19febb2 on use-assert
into db622e3 on master.

RobinTail
RobinTail commented Dec 11, 2023
View reviewed changes
Copy link
Owner Author

@RobinTail RobinTail left a comment

Choose a reason for hiding this comment

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

ready

@RobinTail RobinTail merged commit 1aaa26a into master Dec 11, 2023
@RobinTail RobinTail deleted the use-assert branch December 11, 2023 18:08
This was referenced Nov 15, 2024
Merged
Merged
RobinTail added a commit that referenced this pull request Nov 15, 2024
@RobinTail
Revert: throw instead of assert (#2169)
01b598d 
partially reverts #1371
for performance reasons found in #2168
RobinTail added a commit that referenced this pull request Nov 15, 2024
@RobinTail
Traverse without recursion (#2168)
bc8e9eb 
Avoiding recursion should decrease memory consumption by replacing the
utilization of call stack to a stack of entities to process

https://www.nearform.com/insights/tracking-memory-allocation-node-js/

```
   ✓ Experiment for routing walker (2) 1247ms
     name             hz     min     max    mean     p75     p99    p995    p999     rme  samples
   · current    3,691.20  0.2053  2.7257  0.2709  0.2885  0.3817  0.4519  2.1826  ±1.49%     1846
   · featured  50,253.26  0.0123  5.8032  0.0199  0.0165  0.0423  0.1116  2.7895  ±7.10%    25127   fastest
```

Turned out that the performance is mostly affected by `assert(..., new
Error())` pattern, partially introduced by
#1371
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
refactoring The better way to achieve the same result
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RobinTail @coveralls