mirror of
https://github.com/actions/setup-java.git
synced 2026-06-17 08:54:24 +00:00
Address Copilot code review comments
- Use strict equality (===, !==) instead of loose equality (==, !=) for all comparisons
- Properly handle caught errors with instanceof type narrowing before accessing properties
- Only fall back to legacy AdoptOpenJDK for specific version-not-found errors
- Rethrow unexpected errors to avoid masking real issues (network failures, rate limits, etc.)
- Fix error message check to match actual error text ('No matching version found')
- Remove unnecessary undefined check since method return type is never undefined
- Add @internal JSDoc annotation to TemurinDistribution.findPackageForDownload()
- Update tests to properly mock Temurin lookup failures for fallback behavior testing
- Rebuild dist files
This commit is contained in:
parent
bf12c838ba
commit
7d2d456be4
@ -278,6 +278,10 @@ describe('findPackageForDownload', () => {
|
|||||||
},
|
},
|
||||||
AdoptImplementation.Hotspot
|
AdoptImplementation.Hotspot
|
||||||
);
|
);
|
||||||
|
// Mock Temurin to fail so fallback to AdoptOpenJDK is tested
|
||||||
|
distribution['temurinDistribution']!['findPackageForDownload'] = async () => {
|
||||||
|
throw new Error('No matching version found for SemVer');
|
||||||
|
};
|
||||||
distribution['getAvailableVersions'] = async () => manifestData as any;
|
distribution['getAvailableVersions'] = async () => manifestData as any;
|
||||||
const resolvedVersion = await distribution['findPackageForDownload'](input);
|
const resolvedVersion = await distribution['findPackageForDownload'](input);
|
||||||
expect(resolvedVersion.version).toBe(expected);
|
expect(resolvedVersion.version).toBe(expected);
|
||||||
@ -293,6 +297,10 @@ describe('findPackageForDownload', () => {
|
|||||||
},
|
},
|
||||||
AdoptImplementation.Hotspot
|
AdoptImplementation.Hotspot
|
||||||
);
|
);
|
||||||
|
// Mock Temurin to fail so fallback to AdoptOpenJDK is tested
|
||||||
|
distribution['temurinDistribution']!['findPackageForDownload'] = async () => {
|
||||||
|
throw new Error('No matching version found for SemVer');
|
||||||
|
};
|
||||||
distribution['getAvailableVersions'] = async () => manifestData as any;
|
distribution['getAvailableVersions'] = async () => manifestData as any;
|
||||||
await expect(
|
await expect(
|
||||||
distribution['findPackageForDownload']('9.0.8')
|
distribution['findPackageForDownload']('9.0.8')
|
||||||
@ -309,6 +317,10 @@ describe('findPackageForDownload', () => {
|
|||||||
},
|
},
|
||||||
AdoptImplementation.Hotspot
|
AdoptImplementation.Hotspot
|
||||||
);
|
);
|
||||||
|
// Mock Temurin to fail so fallback to AdoptOpenJDK is tested
|
||||||
|
distribution['temurinDistribution']!['findPackageForDownload'] = async () => {
|
||||||
|
throw new Error('No matching version found for SemVer');
|
||||||
|
};
|
||||||
distribution['getAvailableVersions'] = async () => manifestData as any;
|
distribution['getAvailableVersions'] = async () => manifestData as any;
|
||||||
await expect(distribution['findPackageForDownload']('7.x')).rejects.toThrow(
|
await expect(distribution['findPackageForDownload']('7.x')).rejects.toThrow(
|
||||||
/No matching version found for SemVer */
|
/No matching version found for SemVer */
|
||||||
@ -325,6 +337,10 @@ describe('findPackageForDownload', () => {
|
|||||||
},
|
},
|
||||||
AdoptImplementation.Hotspot
|
AdoptImplementation.Hotspot
|
||||||
);
|
);
|
||||||
|
// Mock Temurin to fail so fallback to AdoptOpenJDK is tested
|
||||||
|
distribution['temurinDistribution']!['findPackageForDownload'] = async () => {
|
||||||
|
throw new Error('No matching version found for SemVer');
|
||||||
|
};
|
||||||
distribution['getAvailableVersions'] = async () => [];
|
distribution['getAvailableVersions'] = async () => [];
|
||||||
await expect(distribution['findPackageForDownload']('11')).rejects.toThrow(
|
await expect(distribution['findPackageForDownload']('11')).rejects.toThrow(
|
||||||
/No matching version found for SemVer */
|
/No matching version found for SemVer */
|
||||||
|
|||||||
44
dist/setup/index.js
vendored
44
dist/setup/index.js
vendored
@ -77815,17 +77815,56 @@ const path_1 = __importDefault(__nccwpck_require__(71017));
|
|||||||
const semver_1 = __importDefault(__nccwpck_require__(11383));
|
const semver_1 = __importDefault(__nccwpck_require__(11383));
|
||||||
const base_installer_1 = __nccwpck_require__(59741);
|
const base_installer_1 = __nccwpck_require__(59741);
|
||||||
const util_1 = __nccwpck_require__(92629);
|
const util_1 = __nccwpck_require__(92629);
|
||||||
|
const installer_1 = __nccwpck_require__(18579);
|
||||||
var AdoptImplementation;
|
var AdoptImplementation;
|
||||||
(function (AdoptImplementation) {
|
(function (AdoptImplementation) {
|
||||||
AdoptImplementation["Hotspot"] = "Hotspot";
|
AdoptImplementation["Hotspot"] = "Hotspot";
|
||||||
AdoptImplementation["OpenJ9"] = "OpenJ9";
|
AdoptImplementation["OpenJ9"] = "OpenJ9";
|
||||||
})(AdoptImplementation || (exports.AdoptImplementation = AdoptImplementation = {}));
|
})(AdoptImplementation || (exports.AdoptImplementation = AdoptImplementation = {}));
|
||||||
class AdoptDistribution extends base_installer_1.JavaBase {
|
class AdoptDistribution extends base_installer_1.JavaBase {
|
||||||
constructor(installerOptions, jvmImpl) {
|
constructor(installerOptions, jvmImpl, temurinDistribution = null) {
|
||||||
super(`Adopt-${jvmImpl}`, installerOptions);
|
super(`Adopt-${jvmImpl}`, installerOptions);
|
||||||
this.jvmImpl = jvmImpl;
|
this.jvmImpl = jvmImpl;
|
||||||
|
this.temurinDistribution = temurinDistribution;
|
||||||
|
if (temurinDistribution !== null &&
|
||||||
|
jvmImpl !== AdoptImplementation.Hotspot) {
|
||||||
|
throw new Error('Only Hotspot JVM is supported by Temurin.');
|
||||||
|
}
|
||||||
|
// Only use the temurin repo for Hotspot JVMs
|
||||||
|
if (temurinDistribution === null &&
|
||||||
|
jvmImpl === AdoptImplementation.Hotspot) {
|
||||||
|
this.temurinDistribution = new installer_1.TemurinDistribution(installerOptions, installer_1.TemurinImplementation.Hotspot);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
findPackageForDownload(version) {
|
findPackageForDownload(version) {
|
||||||
|
return __awaiter(this, void 0, void 0, function* () {
|
||||||
|
if (this.jvmImpl === AdoptImplementation.Hotspot) {
|
||||||
|
core.notice("AdoptOpenJDK has moved to Eclipse Temurin https://github.com/actions/setup-java#supported-distributions please consider changing to the 'temurin' distribution type in your setup-java configuration.");
|
||||||
|
}
|
||||||
|
if (this.jvmImpl === AdoptImplementation.Hotspot &&
|
||||||
|
this.temurinDistribution !== null) {
|
||||||
|
try {
|
||||||
|
return yield this.temurinDistribution.findPackageForDownload(version);
|
||||||
|
}
|
||||||
|
catch (error) {
|
||||||
|
// Only fall back to legacy AdoptOpenJDK for version-not-found errors
|
||||||
|
if (error instanceof Error &&
|
||||||
|
error.message.includes('No matching version found')) {
|
||||||
|
core.notice('The JVM you are looking for could not be found in the Temurin repository, this likely indicates ' +
|
||||||
|
'that you are using an out of date version of Java, consider updating and moving to using the Temurin distribution type in setup-java.');
|
||||||
|
}
|
||||||
|
else {
|
||||||
|
// Rethrow unexpected errors to avoid masking real issues
|
||||||
|
core.debug(`Unexpected error from Temurin lookup: ${error instanceof Error ? error.message : String(error)}`);
|
||||||
|
throw error;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// failed to find a Temurin version, so fall back to AdoptOpenJDK
|
||||||
|
return this.findPackageForDownloadOldAdoptOpenJdk(version);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
findPackageForDownloadOldAdoptOpenJdk(version) {
|
||||||
return __awaiter(this, void 0, void 0, function* () {
|
return __awaiter(this, void 0, void 0, function* () {
|
||||||
const availableVersionsRaw = yield this.getAvailableVersions();
|
const availableVersionsRaw = yield this.getAvailableVersions();
|
||||||
const availableVersionsWithBinaries = availableVersionsRaw
|
const availableVersionsWithBinaries = availableVersionsRaw
|
||||||
@ -80195,6 +80234,9 @@ class TemurinDistribution extends base_installer_1.JavaBase {
|
|||||||
super(`Temurin-${jvmImpl}`, installerOptions);
|
super(`Temurin-${jvmImpl}`, installerOptions);
|
||||||
this.jvmImpl = jvmImpl;
|
this.jvmImpl = jvmImpl;
|
||||||
}
|
}
|
||||||
|
/**
|
||||||
|
* @internal For cross-distribution reuse only. Not intended as a public API.
|
||||||
|
*/
|
||||||
findPackageForDownload(version) {
|
findPackageForDownload(version) {
|
||||||
return __awaiter(this, void 0, void 0, function* () {
|
return __awaiter(this, void 0, void 0, function* () {
|
||||||
const availableVersionsRaw = yield this.getAvailableVersions();
|
const availableVersionsRaw = yield this.getAvailableVersions();
|
||||||
|
|||||||
@ -36,12 +36,18 @@ export class AdoptDistribution extends JavaBase {
|
|||||||
) {
|
) {
|
||||||
super(`Adopt-${jvmImpl}`, installerOptions);
|
super(`Adopt-${jvmImpl}`, installerOptions);
|
||||||
|
|
||||||
if (temurinDistribution != null && jvmImpl != AdoptImplementation.Hotspot) {
|
if (
|
||||||
|
temurinDistribution !== null &&
|
||||||
|
jvmImpl !== AdoptImplementation.Hotspot
|
||||||
|
) {
|
||||||
throw new Error('Only Hotspot JVM is supported by Temurin.');
|
throw new Error('Only Hotspot JVM is supported by Temurin.');
|
||||||
}
|
}
|
||||||
|
|
||||||
// Only use the temurin repo for Hotspot JVMs
|
// Only use the temurin repo for Hotspot JVMs
|
||||||
if (temurinDistribution == null && jvmImpl == AdoptImplementation.Hotspot) {
|
if (
|
||||||
|
temurinDistribution === null &&
|
||||||
|
jvmImpl === AdoptImplementation.Hotspot
|
||||||
|
) {
|
||||||
this.temurinDistribution = new TemurinDistribution(
|
this.temurinDistribution = new TemurinDistribution(
|
||||||
installerOptions,
|
installerOptions,
|
||||||
TemurinImplementation.Hotspot
|
TemurinImplementation.Hotspot
|
||||||
@ -52,30 +58,34 @@ export class AdoptDistribution extends JavaBase {
|
|||||||
protected async findPackageForDownload(
|
protected async findPackageForDownload(
|
||||||
version: string
|
version: string
|
||||||
): Promise<JavaDownloadRelease> {
|
): Promise<JavaDownloadRelease> {
|
||||||
if (this.jvmImpl == AdoptImplementation.Hotspot) {
|
if (this.jvmImpl === AdoptImplementation.Hotspot) {
|
||||||
core.notice(
|
core.notice(
|
||||||
"AdoptOpenJDK has moved to Eclipse Temurin https://github.com/actions/setup-java#supported-distributions please consider changing to the 'temurin' distribution type in your setup-java configuration."
|
"AdoptOpenJDK has moved to Eclipse Temurin https://github.com/actions/setup-java#supported-distributions please consider changing to the 'temurin' distribution type in your setup-java configuration."
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (
|
if (
|
||||||
this.jvmImpl == AdoptImplementation.Hotspot &&
|
this.jvmImpl === AdoptImplementation.Hotspot &&
|
||||||
this.temurinDistribution != null
|
this.temurinDistribution !== null
|
||||||
) {
|
) {
|
||||||
try {
|
try {
|
||||||
const result = await this.temurinDistribution.findPackageForDownload(
|
return await this.temurinDistribution.findPackageForDownload(version);
|
||||||
version
|
|
||||||
);
|
|
||||||
|
|
||||||
if (result != undefined) {
|
|
||||||
return result;
|
|
||||||
}
|
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (error.message.includes('Could not find satisfied version')) {
|
// Only fall back to legacy AdoptOpenJDK for version-not-found errors
|
||||||
|
if (
|
||||||
|
error instanceof Error &&
|
||||||
|
error.message.includes('No matching version found')
|
||||||
|
) {
|
||||||
core.notice(
|
core.notice(
|
||||||
'The JVM you are looking for could not be found in the Temurin repository, this likely indicates ' +
|
'The JVM you are looking for could not be found in the Temurin repository, this likely indicates ' +
|
||||||
'that you are using an out of date version of Java, consider updating and moving to using the Temurin distribution type in setup-java.'
|
'that you are using an out of date version of Java, consider updating and moving to using the Temurin distribution type in setup-java.'
|
||||||
);
|
);
|
||||||
|
} else {
|
||||||
|
// Rethrow unexpected errors to avoid masking real issues
|
||||||
|
core.debug(
|
||||||
|
`Unexpected error from Temurin lookup: ${error instanceof Error ? error.message : String(error)}`
|
||||||
|
);
|
||||||
|
throw error;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -34,6 +34,9 @@ export class TemurinDistribution extends JavaBase {
|
|||||||
super(`Temurin-${jvmImpl}`, installerOptions);
|
super(`Temurin-${jvmImpl}`, installerOptions);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @internal For cross-distribution reuse only. Not intended as a public API.
|
||||||
|
*/
|
||||||
public async findPackageForDownload(
|
public async findPackageForDownload(
|
||||||
version: string
|
version: string
|
||||||
): Promise<JavaDownloadRelease> {
|
): Promise<JavaDownloadRelease> {
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user