This repository has been archived by the owner on Mar 15, 2024. It is now read-only.
Fixes for dog fooding #202
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It is confusing to the user why he cannot use an object for `default-process`: ```yaml process: path: *.js enabled: true default-process: enabled: false ``` I changed to internal representation of processes as well from boolean to the object. It is more explicit and more flexible should we add more information to processes.
NB: `ts-node` fails -- cf: #200
lachrist
force-pushed
the
fix-dog
branch
2 times, most recently
from
80bed7e
to
5c8feeb
Compare
No need to introduce `void 0` shenanigan. We can simply initialize tab variables to `null` instead.
This will help unify yield and await instrumentation.
This is more consistent with the ERROR instrumentation variable.
Better readability of instrumented code.
This is part of the effort to unify yield and await instrumentation. The real motivation is to reuse reject event as well when we will support `Generator.prototype.throw`.
```js
function g* () {
try {
yield 1;
yield 2;
} catch {
// We have to do some cleanup here
// by recording a reject event
yield 3;
yield 4;
}
}
```
resolve instead
There is a slight deviation that does not impact the user. Now, instrumented `yield` expression will cleanup their tab variable. This will be used next for supporting `Generator.prototype.throw` and `Generator.prototype.return` methods.
Bugs related to invalid callstack states are the hardest to diagnose because they are detected during trace post-processing long past their root cause. This assertion should help diagnosing the ones that are related to generator functions and asynchronous functions. To be compact, failure is signaled by reading a missing variable.
This should not matter but in the other callsites of `recordReject` or `recorResolve` we always reset `JUMP_TAB`. Let's be consistent.
`Generator.prototype.throw` allows to throw an error at the location of the last yield expression. We should cleanup the instrumentation variable `JUMP_TAB` in catch blocks to record reject events.
`Generator.prototype.return` allows to return a result at the location of the last yield expression. We should cleanup the instrumentation variable `JUMP_TAB` in finally blocks to record resolve events.
We now always use the esm loader which deals with file extensions better than we did by looking up `type` in nearest `package.json`. I refactored module loading in a dedicated component which simplifies testing. In the future, we might want to use the `load` component to load our own configuration file as well.
I introduced a regression while modifying instrumentation. I rebased it and add this test to prevent this regression in the future.
brikelly
changed the title
brikelly
changed the title
We should use that configuration instead of the cwd of the current process. That enables to spawn jest commands from another directory than the `rootDir` of jest.
apotterri
approved these changes
Mar 13, 2023
There was a problem hiding this comment.
The granularity of the commits in this PR made it pretty tough to review, and will make the history quite hard to follow later.
In particular, it would be much better if a fix or feat commit also contained the changes to test it.
I'll approve this, but you should consider simplifying the history.
|
🎉 This PR is included in version 13.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes and small improvements like to appmapping our own stuff.
#203 is fixed by commit range: