chore(tests): add type annotations to integration test helpers#7240
chore(tests): add type annotations to integration test helpers#7240
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Overall package sizeSelf size: 4.39 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 2.0.0 | 68.46 kB | 797.03 kB | | dc-polyfill | 0.1.10 | 26.73 kB | 26.73 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7240 +/- ##
===========================================
- Coverage 85.13% 72.39% -12.74%
===========================================
Files 532 204 -328
Lines 22654 7840 -14814
===========================================
- Hits 19287 5676 -13611
+ Misses 3367 2164 -1203 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
BenchmarksBenchmark execution time: 2026-01-16 09:17:45 Comparing candidate commit 5d3f175 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 288 metrics, 32 unstable metrics. |
78f4210 to
cde62ed
Compare
09f5e10 to
2fe3616
Compare
cde62ed to
3d6a7b4
Compare
2fe3616 to
02625e4
Compare
3d6a7b4 to
53223a1
Compare
02625e4 to
dbfb772
Compare
53223a1 to
87c16a4
Compare
87c16a4 to
79f75bc
Compare
BridgeAR
left a comment
There was a problem hiding this comment.
Small suggestion for improvement about the port and it weakens some checks, which are probably not problematic, I thought I just ask.
Otherwise LGTM
| if (typeof actualMetadata.result !== 'string') { | ||
| throw new assert.AssertionError({ message: 'result field should be a string' }) | ||
| } | ||
| if (typeof actualMetadata.result_class !== 'string') { |
There was a problem hiding this comment.
These do not verify that the string has to be non empty. Is that intentional?
There was a problem hiding this comment.
My thought was that it was covered by the assertions below, but I see now that actualMetadata.result_reason isn't. I'll update the PR.
Background: The reason why I made this change was because TS wasn't sure that these were strings, so I needed an assertion that checked that they were strings so that TS wouldn't complain when I used them as input to validResults.includes and validResultClasses.includes. I'm not a fan of the way I fixed this, but I couldn't come up with a better approach. But let me know if you have any ideas.
| @@ -217,7 +217,7 @@ function spawnProc (filename, options = {}, stdioHandler, stderrHandler) { | |||
|
|
|||
| return new Promise((resolve, reject) => { | |||
| proc | |||
| .on('message', ({ port }) => { | |||
| .on('message', (/** @type {{ port?: unknown }} */ { port }) => { | |||
There was a problem hiding this comment.
Should port not just be a number (or potentially a string?)
There was a problem hiding this comment.
I used unknown due to the type check below. The type check below is useful for catching runtime errors, so I don't want to remove that. I feels kinda weird to set it to number and then also have the type check. WDYT?
| for (let i = 0; i < args.length; i += 2) { | ||
| expectedPoints.push({ | ||
| name: 'library_entrypoint.' + args[i], | ||
| tags: args[i + 1].split(',').filter(Boolean) | ||
| }) |
79f75bc to
5d3f175
Compare

What does this PR do?
As the title says
Motivation
I wanted to get
integration-tests/helpers/index.jsto a clean state where there's no more type errors.