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.

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

feat(it-tests): test schematics with yarn 1 #1230

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

vscaiceanu-1a
Copy link
Member

@vscaiceanu-1a vscaiceanu-1a commented Jan 16, 2024

Copy link

nx-cloud bot commented Feb 20, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 0f2b6b9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@vscaiceanu-1a vscaiceanu-1a force-pushed the feature/it-tests-yarn1 branch 5 times, most recently from fe76f58 to 9c22f6f Compare February 26, 2024 14:10
@vscaiceanu-1a vscaiceanu-1a force-pushed the feature/it-tests-yarn1 branch 4 times, most recently from ff2c4c6 to 4c6d5fc Compare March 1, 2024 10:32
@vscaiceanu-1a vscaiceanu-1a force-pushed the feature/it-tests-yarn1 branch 4 times, most recently from 995e5fa to 73e5b52 Compare March 4, 2024 14:00
@github-actions github-actions bot added the bug Something isn't working label Mar 13, 2024
@mrednic-1A mrednic-1A force-pushed the feature/it-tests-yarn1 branch 2 times, most recently from 6c27dc6 to fd8f9ca Compare March 15, 2024 15:46
@mrednic-1A mrednic-1A force-pushed the feature/it-tests-yarn1 branch 3 times, most recently from fbb0efc to 232a975 Compare March 27, 2024 13:16
Comment on lines +268 to +271
execFileSync('yarn', ['config', 'set', '@ama-sdk:registry', options.registry], execOptions);
execFileSync('yarn', ['config', 'set', '@ama-terasu:registry', options.registry], execOptions);
execFileSync('yarn', ['config', 'set', '@o3r:registry', options.registry], execOptions);
execFileSync('yarn', ['config', 'set', 'unsafeHttpWhitelist', '127.0.0.1'], execOptions);
Copy link
Contributor

@fpaul-1A fpaul-1A Mar 27, 2024

Choose a reason for hiding this comment

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

if I remember correctly, these are modifying the ~/.yarnrc on the machine, not at project level
this is probably fine for the CI but not ideal when running locally
it might be better to directly modify the file like you do line 274 instead

Copy link
Contributor

@mrednic-1A mrednic-1A Mar 27, 2024

Choose a reason for hiding this comment

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

true that. I'll do the updates directly on the file in the project
EDIT trying to do the change I realized that yarn 1 is using the root yarnrc for the global packages scopes (at least) and not the local one. It;s strange because it is not the case for global/cache folders for instance.

@mrednic-1A mrednic-1A force-pushed the feature/it-tests-yarn1 branch 4 times, most recently from 5800def to 27e6a3a Compare March 27, 2024 19:43

if (options.globalFolderPath) {
if (!content.includes('prefix')) {
appendFileSync(yarnRcPath, `\nprefix "${posix.join(options.globalFolderPath.split(sep).join(posix.sep), `yarn1-global-folder`, options.packageScope || '')}"`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
appendFileSync(yarnRcPath, `\nprefix "${posix.join(options.globalFolderPath.split(sep).join(posix.sep), `yarn1-global-folder`, options.packageScope || '')}"`);
appendFileSync(yarnRcPath, `\nprefix "${posix.join(options.globalFolderPath.split(sep).join(posix.sep), 'yarn1-global-folder', options.packageScope || '')}"`);

Comment on lines +288 to +286
['cache-folder', 'global-folder'].forEach(folder => {
if (!content.includes(folder)) {
appendFileSync(yarnRcPath, `\n${folder} "${posix.join(options.globalFolderPath!.split(sep).join(posix.sep), `yarn1-${folder}`, options.packageScope || '')}"`);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
['cache-folder', 'global-folder'].forEach(folder => {
if (!content.includes(folder)) {
appendFileSync(yarnRcPath, `\n${folder} "${posix.join(options.globalFolderPath!.split(sep).join(posix.sep), `yarn1-${folder}`, options.packageScope || '')}"`);
}
});
['cache-folder', 'global-folder']
.filter((folder) => !content.includes(folder))
.forEach(folder => {
appendFileSync(yarnRcPath, `\n${folder} "${posix.join(options.globalFolderPath!.split(sep).join(posix.sep), `yarn1-${folder}`, options.packageScope || '')}"`);
});

* 'yarn', 'npm'
*/
export function getPackageManager() {
return getVersionedPackageManager() === 'npm' ? 'npm' : 'yarn';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return getVersionedPackageManager() === 'npm' ? 'npm' : 'yarn';
const pm = getVersionedPackageManager();
return pm.startsWith('yarn') ? 'yarn' : pm;

@mrednic-1A mrednic-1A force-pushed the feature/it-tests-yarn1 branch 3 times, most recently from 3aa9e25 to 309e8f4 Compare March 29, 2024 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All the schematics should be tested in yarn 1.* [Bug]: yarn create not working for Yarn 1.*
5 participants