fix(selectivity): ensure selectivity dump has no stale data#1214
fix(selectivity): ensure selectivity dump has no stale data#1214KuznetsovRoman wants to merge 2 commits intomasterfrom
Conversation
commit: |
KuznetsovRoman
force-pushed
the
TESTPLANE-921.clear_dump_unused
branch
from
2b2b3bd to
2527862
Compare
KuznetsovRoman
force-pushed
the
TESTPLANE-921.clear_dump_unused
branch
from
2527862 to
19810a9
Compare
| dependencies.modules.forEach(dependency => this._addModuleDependency(dependency)); | ||
| } | ||
|
|
||
| async commit(): Promise<void> { |
There was a problem hiding this comment.
Removing logic to "not remove anything extra"
It was useful when hash writes were concurrent
| } | ||
|
|
||
| // File was not used in this run | ||
| if (fileStat.atimeMs < performance.timeOrigin && fileStat.isFile()) { |
There was a problem hiding this comment.
"atime" - time file was accessed
"clearUnusedSelectivityDumps" is called after successful run
If file was not accessed in successful run, we can safely delete it
| try { | ||
| const stats = await libUVLimited(() => fs.stat(flagFilePath)); | ||
| return stats.mtimeMs >= processStartTime; | ||
| return stats.mtimeMs >= performance.timeOrigin; |
There was a problem hiding this comment.
Turns out, there is special value for that, so we dont need "new Date()" hacks
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19810a9bc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await writeTo(this._stagedFileHashes, fileContents.files); | ||
| await writeTo(this._stagedModuleHashes, fileContents.modules); | ||
| await writeTo(this._stagedPatternHashes, fileContents.patterns); | ||
|
|
||
| markAsCommited(updatedModules, this._stagedModuleHashes); | ||
| markAsCommited(updatedFiles, this._stagedFileHashes); | ||
| await writeJsonWithCompression(this._selectivityHashesPath, fileContents, this._compresion); |
There was a problem hiding this comment.
HashWriter.save now writes a brand-new hash object populated only from currently staged entries, but staging is filled only from executed tests (onTestDependencies) and changed disable patterns (updateSelectivityHashes). This drops hashes for skipped tests and unchanged patterns, so on the next process run HashReader.getTestChangedDeps/patternHasChanged read undefined and treat those dependencies as changed, causing selectivity to rerun tests (or whole browsers) even when source files are unchanged.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
now writes a brand-new hash object populated only from currently staged entries
Yes.
but staging is filled only from executed tests
No. It is also filled in master thread while calculating current file hashes.
and changed disable patterns
Unchanged too. For same reasons.
| } | ||
|
|
||
| // File was not used in this run | ||
| if (fileStat.atimeMs < performance.timeOrigin && fileStat.isFile()) { |
There was a problem hiding this comment.
The stale-file check deletes dumps based on fileStat.atimeMs < performance.timeOrigin, but on filesystems mounted with relatime/noatime a read during this run may not update atime, so actively used dependency dumps can still look stale and get removed. In those environments, the cleanup step can delete valid selectivity dumps and force unnecessary full reruns on the next execution.
Useful? React with 👍 / 👎.
1 participant
No description provided.