feat(experimental): implement ability to write test dependencies#1137
Conversation
commit: |
| this._sourceRoot = sourceRoot; | ||
| } | ||
|
|
||
| private _processStyle({ header: { styleSheetId, sourceURL, sourceMapURL } }: CssEvents["styleSheetAdded"]): void { |
There was a problem hiding this comment.
handles emitted "CSS.styleSheetAdded" events
| import fs from "node:fs"; | ||
|
|
||
| export class FileHashProvider { | ||
| private static readonly _hashStore: Map<string, Promise<string>> = new Map(); |
There was a problem hiding this comment.
assuming hash for file can't be changed during Testplane launch
| import { shallowSortObject } from "./utils"; | ||
| import type { NormalizedDependencies } from "./types"; | ||
|
|
||
| export class FileHashWriter { |
There was a problem hiding this comment.
tracks up-to-date hashes for all files:
| if (!cdpTargetId) { | ||
| throw new Error( |
There was a problem hiding this comment.
I dont think this error could ever be thrown, though
| this._sourceRoot = sourceRoot; | ||
| } | ||
|
|
||
| private _processScript({ scriptId, url, sourceMapURL }: DebuggerEvents["scriptParsed"]): void { |
There was a problem hiding this comment.
handles emitted "scriptParsed" events
| }; | ||
|
|
||
| const ensurePosixRelativeDependencyPathExists = memoize((posixRelativePath: string): void => { | ||
| const relativePath = posixRelativePath.replaceAll(path.posix.sep, path.sep); |
There was a problem hiding this comment.
source map sources are posix paths (use "/" as separator even on windows)
| const modules: string[] = []; | ||
|
|
||
| const classifyDependency = (dependency: string, typedResultArray: string[]): void => { | ||
| dependency = decodeURIComponent(softFileURLToPath(dependency)); |
There was a problem hiding this comment.
"dependency" from source-map "sources" are url-like
So it can have "file://" protocol. It also can be relative path with urlencoded symbols
| throw new Error(`Selectivity: Found unsupported protocol in dependencies ("${dependency}")`); | ||
| } | ||
|
|
||
| const dependencyRelativePath = path.posix.relative(path.posix.resolve(), path.posix.resolve(dependency)); |
There was a problem hiding this comment.
- using "path.posix.resolve()" to get posix cwd
- "path.posix.resolve(dependency)" resolves absolute path from cwd
| let cssIndex = 0; | ||
| let jsIndex = 0; |
There was a problem hiding this comment.
using 2 pointers to create merged "modules" array of uniq modules from sorted "css" and "js" arrays
| // Ensures file consistency | ||
| export const shallowSortObject = (obj: Record<string, unknown>): void => { |
There was a problem hiding this comment.
used before writing object to file system to ensure keys order is stable
| } | ||
|
|
||
| const hash = new Promise<string>((resolve, reject) => { | ||
| const hash = crypto.createHash("md5"); |
There was a problem hiding this comment.
using md5 hashes as we dont need hash to be crypto-strong, and md5 is fast
KuznetsovRoman
force-pushed
the
TESTPLANE-668.selectivity_p1_write
branch
from
862d56a to
646bf8d
Compare
KuznetsovRoman
force-pushed
the
TESTPLANE-668.selectivity_p1_write
branch
3 times, most recently
from
db6f13c to
b1016a2
Compare
shadowusr
left a comment
shadowusr
left a comment
There was a problem hiding this comment.
The most important thing is the docs on how this feature can be used along with complete example how this solves user's scenario. I think this is very important. Without docs and examples, I think its hardly possible to use.
A nice addition overall! 🔥
| sourceMapsEndIndex, | ||
| ); | ||
|
|
||
| const isSourceMapEmbedded = new URL(sourceMapURL).protocol === "data:"; |
There was a problem hiding this comment.
Hmmm wouldn't this throw if sourceMapURL is an absolute or relative path? Or it's guaranteed to have an URL format?
There was a problem hiding this comment.
By specification, its url-like:
The sources field is a list of original sources used by the mappings field. Each entry is either a string that is a (potentially relative) URL or null if the source name is not known
| } | ||
|
|
||
| if (!browser.cdp) { | ||
| throw new Error("Selectivity: Devtools connection is not established, couldn't record selectivity without it"); |
There was a problem hiding this comment.
not critical: when can this happen and why exactly? can we give some advice here?
There was a problem hiding this comment.
when can this happen and why exactly?
No idea
not critical: when can this happen and why exactly? can we give some advice here?
No, since we dont know, why cdp connection might not be established, while we see the cases
There was a problem hiding this comment.
As far as I understood, that hashes.json file can be used to detect which files changed, fast. But I don't see how it's used later. Is it used anywhere? Can we leave a short comment here about the purpose of this class?
There was a problem hiding this comment.
in part 1 (this pr), we only write dependencies, and dont read them
"read deps" + "skip unchanged" is part 2
| reuseExisting: boolean; | ||
| }; | ||
|
|
||
| selectivity: { |
There was a problem hiding this comment.
This type has the same issue as many other sections in our config — can't just enable it and be fine with defaults. It forces me to specify all of the fields, even though they have defaults and it shouldn't.
| return sourceMap; | ||
| }; | ||
|
|
||
| export const extractSourceFilesDeps = async ( |
There was a problem hiding this comment.
not critical — some of these helpers contain a pretty complicated logic and just judging by their name it's hard to understand what exactly they do. I would add an explanatory comment like
Given compiled code, its source map, and the executed offsets, it returns the original source files touched — useful for turning coverage ranges into real TS/JS module dependencies.
KuznetsovRoman
force-pushed
the
TESTPLANE-668.selectivity_p1_write
branch
from
b1016a2 to
6a85f19
Compare
2 participants
No description provided.