Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved calculating pre-release versions that start with the same base name #1064

Merged
merged 5 commits into from
Feb 10, 2025
Merged

Improved calculating pre-release versions that start with the same base name #1064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
57be9b8
Exported `getDateIdentifier()` from the `ckeditor5-dev-release-tools`…
psmyrek Feb 6, 2025
06114d0
Improved mocked data in test.
psmyrek Feb 6, 2025
20be140
Tests.
psmyrek Feb 7, 2025
dc597e1
Tests.
psmyrek Feb 7, 2025
b90cd72
Match the dot in the optional sequence number literally.
psmyrek Feb 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/ckeditor5-dev-release-tools/lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export {
getNextNightly,
getNextInternal,
getCurrent,
getDateIdentifier,
getLastTagFromGit
} from './utils/versions.js';
export { default as getChangesForVersion } from './utils/getchangesforversion.js';
Expand Down
24 changes: 17 additions & 7 deletions packages/ckeditor5-dev-release-tools/lib/utils/versions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,15 @@ export function getLastFromChangelog( cwd = process.cwd() ) {

/**
* Returns the current (latest) pre-release version that matches the provided release identifier.
* It takes into account and distinguishes pre-release tags with different names but starting with the same base name.
* If the package does not have any pre-releases with the provided identifier yet, `null` is returned.
*
* Examples:
* * "0.0.0-nightly" - Matches the last "nightly" version regardless of the publication date.
* It does not match other nightly tags that start with the same "nightly" base name, e.g. "0.0.0-nightly-next-YYYYMMDD.X".
* * "0.0.0-nightly-20230615" - Matches the last "nightly" version from the 2023-06-15 day.
* * "42.0.0-alpha" - Matches the last "alpha" version for the 42.0.0 version.
*
* @param {ReleaseIdentifier} releaseIdentifier
* @param {string} [cwd=process.cwd()]
* @returns {Promise.<string|null>}
Expand All @@ -41,7 +48,13 @@ export function getLastPreRelease( releaseIdentifier, cwd = process.cwd() ) {
return packument( packageName )
.then( result => {
const lastVersion = Object.keys( result.versions )
.filter( version => version.startsWith( releaseIdentifier ) )
.filter( version => {
const optionalDateIdentifier = '(-[0-9]{8})?';
const optionalSequenceNumber = '(\\.[0-9]+)?';
const versionRegExp = new RegExp( `^${ releaseIdentifier }${ optionalDateIdentifier }${ optionalSequenceNumber }$` );

return versionRegExp.test( version );
Copy link
Member

@filipsobol filipsobol Feb 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a random idea, but maybe we should separate all bits that are important to us by . instead of a mix of . and -?

Given this example:

0.0.0-nightly-next-20230615.0
                  ^        ^

This version number consists of:

MAJOR.MINOR.PATCH-channel_and_date.version

However, according to spec, each identifier in the pre-release portion (after first -) should be dot-separated, so the correct version should be:

0.0.0-nightly-next.20230615.0
                  ^ `.` instead of `-`

This version number consists of:

MAJOR.MINOR.PATCH-channel.date.version

With this approach, we could use semver (which we already have) to parse version numbers instead of using Regexp.

import semver from 'semver';

const data = semver.parse( '0.0.0-nightly-next.20230615.0' );

/**
 * data.raw = '0.0.0-nightly-next.20230615'
 * data.major = 0
 * data.minor = 0
 * data.patch = 0
 * data.prerelease = [ 'nightly-next', '20230615', 0 ]
 */

If this is the only place we do it, then it shouldn't be an issue, but if we have more such places, then relying on official npm tooling would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, due to the fact we are separating nightly-YYYYMMDD with - instead of ., we can't use semver.prerelease() to parse the version, so currently we are doing it manually with regexp.

So question is do we accept to change the versioning for all nightly releases:

  • 0.0.0-nightly-YYYYMMDD.X -> 0.0.0-nightly.YYYYMMDD.X
  • 0.0.0-nightly-next-YYYYMMDD.X -> 0.0.0-nightly.next.YYYYMMDD.X

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine with it, but I wonder if it's a breaking change. Or, whether it should be marked as.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing this within the team, we decided not to change the nightly versioning format.

} )
.sort( ( a, b ) => a.localeCompare( b, undefined, { numeric: true } ) )
.pop();

Expand Down Expand Up @@ -138,9 +151,11 @@ export function getCurrent( cwd = process.cwd() ) {
}

/**
* Returns current date in the "YYYYMMDD" format.
*
* @returns {string}
*/
function getDateIdentifier() {
export function getDateIdentifier() {
const today = new Date();
const year = today.getFullYear().toString();
const month = ( today.getMonth() + 1 ).toString().padStart( 2, '0' );
Expand All @@ -152,9 +167,4 @@ function getDateIdentifier() {
/**
* @typedef {string} ReleaseIdentifier The pre-release identifier without the last dynamic part (the pre-release sequential number).
* It consists of the core base version ("<major>.<minor>.<path>"), a hyphen ("-"), and a pre-release identifier name (e.g. "alpha").
*
* Examples:
* * "0.0.0-nightly" - matches the last nightly version regardless of the publication date.
* * "0.0.0-nightly-20230615" - matches the last nightly version from the 2023-06-15 day.
* * "42.0.0-alpha" - matches the last alpha version for the 42.0.0 version.
*/
8 changes: 8 additions & 0 deletions packages/ckeditor5-dev-release-tools/tests/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
getNextNightly,
getNextInternal,
getCurrent,
getDateIdentifier,
getLastTagFromGit
} from '../lib/utils/versions.js';
import executeInParallel from '../lib/utils/executeinparallel.js';
Expand Down Expand Up @@ -150,6 +151,13 @@ describe( 'dev-release-tools/index', () => {
} );
} );

describe( 'getDateIdentifier()', () => {
it( 'should be a function', () => {
expect( getDateIdentifier ).to.be.a( 'function' );
expect( index.getDateIdentifier ).to.equal( getDateIdentifier );
} );
} );

describe( 'getLastPreRelease()', () => {
it( 'should be a function', () => {
expect( getLastPreRelease ).to.be.a( 'function' );
Expand Down
168 changes: 163 additions & 5 deletions packages/ckeditor5-dev-release-tools/tests/utils/versions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
getNextNightly,
getNextInternal,
getLastTagFromGit,
getCurrent
getCurrent,
getDateIdentifier
} from '../../lib/utils/versions.js';

vi.mock( '@ckeditor/ckeditor5-dev-utils' );
Expand Down Expand Up @@ -145,6 +146,23 @@ describe( 'versions', () => {
} );
} );

it( 'returns null if pre-release version matches the release identifier only partially', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.0': {},
'0.0.0-nightly-20230615.1': {},
'0.0.0-nightly-20230615.2': {},
'0.0.0-nightly-next-20230615.3': {}
}
} );

return getLastPreRelease( '0.0.0-nightly-2023' )
.then( result => {
expect( result ).to.equal( null );
} );
} );

it( 'returns last pre-release version matching the release identifier', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
Expand Down Expand Up @@ -245,14 +263,119 @@ describe( 'versions', () => {
expect( result ).to.equal( '0.0.0-nightly-20230615.2' );
} );
} );

it( 'returns last version from exactly the "nightly" tag when multiple nightly tags exist', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.0': {},
'0.0.0-nightly-20230615.1': {},
'0.0.0-nightly-20230615.2': {},
'0.0.0-nightly-next-20230615.3': {}
}
} );

return getLastPreRelease( '0.0.0-nightly' )
.then( result => {
expect( result ).to.equal( '0.0.0-nightly-20230615.2' );
} );
} );

it( 'returns last version from exactly the "nightly-next" tag when multiple nightly tags exist', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-next-20230615.0': {},
'0.0.0-nightly-next-20230615.1': {},
'0.0.0-nightly-next-20230615.2': {},
'0.0.0-nightly-20230615.3': {}
}
} );

return getLastPreRelease( '0.0.0-nightly-next' )
.then( result => {
expect( result ).to.equal( '0.0.0-nightly-next-20230615.2' );
} );
} );

it( 'returns last version from exactly the "nightly" tag when multiple nightly tags exist from a specific day', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.0': {},
'0.0.0-nightly-20230615.1': {},
'0.0.0-nightly-20230615.2': {},
'0.0.0-nightly-next-20230615.3': {}
}
} );

return getLastPreRelease( '0.0.0-nightly-20230615' )
.then( result => {
expect( result ).to.equal( '0.0.0-nightly-20230615.2' );
} );
} );

it( 'returns last version from exactly the "nightly-next" tag when multiple nightly tags exist from a specific day', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-next-20230615.0': {},
'0.0.0-nightly-next-20230615.1': {},
'0.0.0-nightly-next-20230615.2': {},
'0.0.0-nightly-20230615.3': {}
}
} );

return getLastPreRelease( '0.0.0-nightly-next-20230615' )
.then( result => {
expect( result ).to.equal( '0.0.0-nightly-next-20230615.2' );
} );
} );

it( 'returns last pre-release version matching the release identifier exactly', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.0': {},
'37.0.0-alpha.1': {},
'37.0.0-alpha.2': {},
'41.0.0': {},
'37.0.0-alpha.10': {},
'37.0.0-alpha.11': {}
}
} );

return getLastPreRelease( '37.0.0-alpha.10' )
.then( result => {
expect( result ).to.equal( '37.0.0-alpha.10' );
} );
} );

it( 'returns last nightly version matching the release identifier exactly', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.0': {},
'0.0.0-nightly-20230615.1': {},
'0.0.0-nightly-20230615.2': {},
'0.0.0-nightly-next-20230615.1': {},
'0.0.0-nightly-next-20230615.2': {}
}
} );

return getLastPreRelease( '0.0.0-nightly-20230615.1' )
.then( result => {
expect( result ).to.equal( '0.0.0-nightly-20230615.1' );
} );
} );
} );

describe( 'getLastNightly()', () => {
beforeEach( async () => {
vi.mocked( getPackageJson ).mockReturnValue( { name: 'ckeditor5' } );
} );

it( 'returns last nightly pre-release version', () => {
it( 'returns last pre-release version from exactly the "nightly" tag', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
Expand All @@ -261,6 +384,7 @@ describe( 'versions', () => {
'0.0.0-nightly-20230614.1': {},
'0.0.0-nightly-20230614.2': {},
'0.0.0-nightly-20230615.0': {},
'0.0.0-nightly-next-20230616.0': {},
'37.0.0-alpha.0': {},
'42.0.0': {}
}
Expand Down Expand Up @@ -310,11 +434,12 @@ describe( 'versions', () => {
} );
} );

it( 'returns nightly version with incremented id if older nightly version was already published', () => {
it( 'returns version with incremented id from exactly the "nightly" tag if older version was already published', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.5': {},
'0.0.0-nightly-next-20230616.0': {},
'37.0.0-alpha.0': {},
'42.0.0': {}
}
Expand All @@ -325,6 +450,23 @@ describe( 'versions', () => {
expect( result ).to.equal( '0.0.0-nightly-20230615.6' );
} );
} );

it( 'returns version with incremented id from exactly the "nightly-next" tag if older version was already published', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-next-20230615.5': {},
'0.0.0-nightly-20230616.0': {},
'37.0.0-alpha.0': {},
'42.0.0': {}
}
} );

return getNextPreRelease( '0.0.0-nightly-next' )
.then( result => {
expect( result ).to.equal( '0.0.0-nightly-next-20230615.6' );
} );
} );
} );

describe( 'getNextNightly()', () => {
Expand All @@ -339,11 +481,12 @@ describe( 'versions', () => {
vi.useRealTimers();
} );

it( 'asks for a last nightly pre-release version', () => {
it( 'returns next pre-release version from exactly the "nightly" tag', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
'0.0.0-nightly-20230615.0': {},
'0.0.0-nightly-next-20230615.5': {},
'37.0.0-alpha.0': {},
'42.0.0': {}
}
Expand All @@ -368,7 +511,7 @@ describe( 'versions', () => {
vi.useRealTimers();
} );

it( 'asks for a last internal pre-release version', () => {
it( 'returns next internal pre-release version', () => {
vi.mocked( packument ).mockResolvedValue( {
name: 'ckeditor5',
versions: {
Expand All @@ -385,6 +528,21 @@ describe( 'versions', () => {
} );
} );

describe( 'getDateIdentifier()', () => {
beforeEach( () => {
vi.useFakeTimers();
vi.setSystemTime( new Date( '2023-06-15 12:00:00' ) );
} );

afterEach( () => {
vi.useRealTimers();
} );

it( 'returns current date in the YYYYMMDD format', () => {
expect( getDateIdentifier() ).to.equal( '20230615' );
} );
} );

describe( 'getLastTagFromGit()', () => {
it( 'returns last tag if exists', () => {
vi.mocked( tools.shExec ).mockReturnValue( 'v1.0.0' );
Expand Down