Skip to content

Commit 807a168

Browse files
LeSingh1iclanton
andauthored
[rush] Include seconds in generated change file names so repeated "rush change" runs do not collide (#5818)
* [rush] Include seconds in generated change file names Running rush change twice in the same minute silently overwrote the change file from the first run, since generatePath only used minute granularity. Pass useSeconds so the filename includes seconds, and fix the useSeconds branch which only replaced the first colon. Fixes #2195 * [rush] Fix ChangeFile test so it passes on Windows with Node.js v24 The Date constructor spy was bypassed by the V8 fast-path on Windows/Node v24, so new Date().toJSON() returned the real wall-clock time instead of the pinned value. Switch to jest.useFakeTimers().setSystemTime() which intercepts at the engine level and is reliable across platforms. Also fix the not.toContain(':') assertion to check only path.basename() instead of the full path. On Windows the generated path starts with a drive letter (e.g. "D:\...") which legitimately contains a colon and was causing the assertion to fail even though the timestamp itself had no colons. * Keep the Date spy and only scope the colon check to the filename The earlier switch to jest fake timers froze time for the rest of the suite and broke the test on every platform. The Date constructor spy already pins the timestamp correctly, as the original Windows failure showed the right pinned value. The only real problem was that the colon check looked at the full path, which on Windows starts with a drive letter like D: that legitimately contains a colon. Restore the spy and keep the assertion scoped to the basename. * Rush change. Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com> * Rush change. Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com> * Clean up jest mocking in the new test. --------- Co-authored-by: Ian Clanton-Thuon <iclanton@users.noreply.github.com>
1 parent 7991f4d commit 807a168

3 files changed

Lines changed: 52 additions & 5 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"comment": "Include seconds in the generated change file name so that running `rush change` more than once in the same minute no longer silently overwrites the previously generated change file.",
5+
"type": "minor",
6+
"packageName": "@microsoft/rush"
7+
}
8+
],
9+
"packageName": "@microsoft/rush",
10+
"email": "LeSingh1@users.noreply.github.com"
11+
}

libraries/rush-lib/src/api/ChangeFile.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,10 +84,15 @@ export class ChangeFile {
8484
console.log('Could not automatically detect git branch name, using timestamp instead.');
8585
}
8686

87-
// example filename: yourbranchname_2017-05-01-20-20.json
87+
// The timestamp includes seconds so that running "rush change" more than once
88+
// in the same minute produces distinct filenames. Without this, the "--overwrite"
89+
// flag rarely had any effect, and a second invocation would silently clobber the
90+
// change file written by the first one. See GitHub issue #2195.
91+
// example filename: yourbranchname_2017-05-01-20-20-30.json
92+
const timestamp: string | undefined = this._getTimestamp(true);
8893
const filename: string = branch
89-
? this._escapeFilename(`${branch}_${this._getTimestamp()}.json`)
90-
: `${this._getTimestamp()}.json`;
94+
? this._escapeFilename(`${branch}_${timestamp}.json`)
95+
: `${timestamp}.json`;
9196
const filePath: string = path.join(
9297
this._rushConfiguration.changesFolder,
9398
...this._changeFileData.packageName.split('/'),
@@ -98,7 +103,7 @@ export class ChangeFile {
98103

99104
/**
100105
* Gets the current time, formatted as YYYY-MM-DD-HH-MM
101-
* Optionally will include seconds
106+
* When useSeconds is true, the seconds are appended as well: YYYY-MM-DD-HH-MM-SS
102107
*/
103108
private _getTimestamp(useSeconds: boolean = false): string | undefined {
104109
// Create a date string with the current time
@@ -120,7 +125,7 @@ export class ChangeFile {
120125
let formattedTime: string;
121126
if (useSeconds) {
122127
// formattedTime === "22-47-49"
123-
formattedTime = matches[2].replace(':', '-');
128+
formattedTime = matches[2].replace(/:/g, '-');
124129
} else {
125130
// formattedTime === "22-47"
126131
const timeParts: string[] = matches[2].split(':');

libraries/rush-lib/src/api/test/ChangeFile.test.ts

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,42 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
22
// See LICENSE in the project root for license information.
33

4+
import type { GitRepoInfo } from 'git-repo-info';
5+
46
import { ChangeFile } from '../ChangeFile';
57
import { RushConfiguration } from '../RushConfiguration';
68
import { ChangeType } from '../ChangeManagement';
9+
import { Git } from '../../logic/Git';
710

811
describe(ChangeFile.name, () => {
12+
it('generates a path that includes seconds so repeated invocations do not collide', () => {
13+
const rushFilename: string = `${__dirname}/repo/rush-npm.json`;
14+
const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(rushFilename);
15+
16+
// Pin the branch name so the generated filename is deterministic.
17+
jest.spyOn(Git.prototype, 'getGitInfo').mockReturnValue({ branch: 'my-branch' } as Readonly<GitRepoInfo>);
18+
19+
// Pin the clock to 2017-05-01 20:20:30 UTC so the timestamp is deterministic.
20+
jest.useFakeTimers({
21+
now: new Date('2017-05-01T20:20:30.000Z').getTime()
22+
});
23+
24+
const changeFile: ChangeFile = new ChangeFile(
25+
{
26+
packageName: 'a',
27+
changes: [],
28+
email: 'fake@example.com'
29+
},
30+
rushConfiguration
31+
);
32+
33+
const generatedPath: string = changeFile.generatePath();
34+
// The seconds must be present and the filename must be fully dash-separated
35+
// (no leftover colons from the time portion).
36+
// Check toContain on the forward-slash-normalised path so it works on Windows too.
37+
expect(generatedPath.replace(/\\/g, '/').endsWith('my-branch_2017-05-01-20-20-30.json')).toBe(true);
38+
});
39+
940
it('can add a change', () => {
1041
const rushFilename: string = `${__dirname}/repo/rush-npm.json`;
1142
const rushConfiguration: RushConfiguration = RushConfiguration.loadFromConfigurationFile(rushFilename);

0 commit comments

Comments
 (0)