chore(import): fix failing tests on windows os

[amorscher]

Fixes failing import unit tests on windows operating system

Description

Fixes failing import unit tests on windows operating system

Motivation and Context

Re-activate ignored unit tests to achieve higher test coverage and regression testing.

How Has This Been Tested?

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (change that has absolutely no effect on users)

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[nx-cloud]

☁️ Nx Cloud Report

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

📂 See all runs for this branch

---

✅ Successfully ran 8 targets

• nx prepare-for-e2e e2e-run-task-runner
• nx run integration:integration --ci --maxWorkers=2
• nx run-many --t e2e --parallel=1
• nx run-many -t build --parallel=3
• nx run-many -t lint --parallel=3
• nx run-many -t test --parallel=3 --ci --maxWorkers=2
• nx run-many -t test --parallel=3 --ci --maxWorkers=2 --testTimeout=60000
• nx-cloud record -- npx nx format:check

---

Sent with 💌 from NxCloud.

[amorscher]

@fahslaj @JamesHenry I found the problem. It is related to different temp dirs on the main and the agents. So on the main we have C:/Users/runneradmin/AppData/Local/Temp/69d67553712c3557d7449c90945ed8ba/test on the agent we have C:\Users\RUNNER~1\AppData\Local\Temp\69d67553712c3557d7449c90945ed8ba\test. So then the import command tries to extract the relative part out of gitRepoRoot and the project root. But actually the project root is not part of the workspace root. See the code here:

lerna/libs/commands/import/src/index.ts

Lines 71 to 76 in f5bd591

	// Compute a target directory relative to the Git root
	const gitRepoRoot = this.getWorkspaceRoot();
	const lernaRootRelativeToGitRoot = path.relative(gitRepoRoot, this.project.rootPath);
	this.targetDirRelativeToGitRoot = path.join(lernaRootRelativeToGitRoot, targetDir);
	if (fs.existsSync(path.resolve(this.project.rootPath, targetDir))) {

We then get something like this ..\..\..\..\..\..\RUNNER~1\AppData\Local\Temp\69d67553712c3557d7449c90945ed8ba\test as the targetDirRelativeToGitRoot . This used to create the import patch which results in an "empty" import as the paths are not considered to be in the git repo. See here:

lerna/libs/commands/import/src/index.ts

Lines 181 to 192 in f5bd591

	const formattedTarget = this.targetDirRelativeToGitRoot.replace(/\\/g, "/");
	const replacement = `$1/${formattedTarget}`;
	// Create a patch file for this commit and prepend the target directory
	// to all affected files. This moves the git history for the entire
	// external repository into the package subdirectory, commit by commit.
	return patch
	.replace(/^([-+]{3} "?COMPARE_[AB])/gm, replacement)
	.replace(/^(diff --git "?COMPARE_A)/gm, replacement)
	.replace(/^(diff --git (?! "?COMPARE_B\/).+ "?COMPARE_B)/gm, replacement)
	.replace(/^(copy (from|to)) ("?)/gm, `$1 $3${formattedTarget}/`)
	.replace(/^(rename (from|to)) ("?)/gm, `$1 $3${formattedTarget}/`);

So from the import point of view everthing seems to be ok but actually nothing is imported. This can also happen if you have symlinks involved.

To slove this I just set the temp dir in the ci config to C:\temp.
In addition I think I will add a check on this to ensure that the projectRoot is part of the workspaceRoot.
What do you think?

[fahslaj]

@fahslaj @JamesHenry I found the problem. It is related to different temp dirs on the main and the agents. So on the main we have C:/Users/runneradmin/AppData/Local/Temp/69d67553712c3557d7449c90945ed8ba/test on the agent we have C:\Users\RUNNER~1\AppData\Local\Temp\69d67553712c3557d7449c90945ed8ba\test. So then the import command tries to extract the relative part out of gitRepoRoot and the project root. But actually the project root is not part of the workspace root. See the code here:

lerna/libs/commands/import/src/index.ts

Lines 71 to 76 in f5bd591

	// Compute a target directory relative to the Git root
	const gitRepoRoot = this.getWorkspaceRoot();
	const lernaRootRelativeToGitRoot = path.relative(gitRepoRoot, this.project.rootPath);
	this.targetDirRelativeToGitRoot = path.join(lernaRootRelativeToGitRoot, targetDir);
	if (fs.existsSync(path.resolve(this.project.rootPath, targetDir))) {

We then get something like this ..\..\..\..\..\..\RUNNER~1\AppData\Local\Temp\69d67553712c3557d7449c90945ed8ba\test as the targetDirRelativeToGitRoot . This used to create the import patch which results in an "empty" import as the paths are not considered to be in the git repo. See here:

lerna/libs/commands/import/src/index.ts

Lines 181 to 192 in f5bd591

	const formattedTarget = this.targetDirRelativeToGitRoot.replace(/\\/g, "/");
	const replacement = `$1/${formattedTarget}`;
	// Create a patch file for this commit and prepend the target directory
	// to all affected files. This moves the git history for the entire
	// external repository into the package subdirectory, commit by commit.
	return patch
	.replace(/^([-+]{3} "?COMPARE_[AB])/gm, replacement)
	.replace(/^(diff --git "?COMPARE_A)/gm, replacement)
	.replace(/^(diff --git (?! "?COMPARE_B\/).+ "?COMPARE_B)/gm, replacement)
	.replace(/^(copy (from|to)) ("?)/gm, `$1 $3${formattedTarget}/`)
	.replace(/^(rename (from|to)) ("?)/gm, `$1 $3${formattedTarget}/`);

So from the import point of view everthing seems to be ok but actually nothing is imported. This can also happen if you have symlinks involved.

To slove this I just set the temp dir in the ci config to C:\temp. In addition I think I will add a check on this to ensure that the projectRoot is part of the workspaceRoot. What do you think?

What do you mean by "ensure that the projectRoot is part of the workspaceRoot"? Are you talking about adding this as functionality of Lerna, or just an additional check within the tests themselves?

[amorscher]

Well I mean here in the lerna import command:

lerna/libs/commands/import/src/index.ts

Lines 71 to 79 in f5bd591

	// Compute a target directory relative to the Git root
	const gitRepoRoot = this.getWorkspaceRoot();
	const lernaRootRelativeToGitRoot = path.relative(gitRepoRoot, this.project.rootPath);
	this.targetDirRelativeToGitRoot = path.join(lernaRootRelativeToGitRoot, targetDir);
	if (fs.existsSync(path.resolve(this.project.rootPath, targetDir))) {
	throw new ValidationError("EEXISTS", `Target directory already exists "${targetDir}"`);
	}

Something like the following after L75

if (this.targetDirRelativeToGitRoot.startsWith("..")) {
      throw new ValidationError("NOTINREPOSITORY", `Project root ${this.project.rootPath} is a subdirectory of git root ${gitRepoRoot}`);
}

[amorscher]

@fahslaj
hey Austin,

Is there anything missing from your point of view? Thx.

[amorscher]

@fahslaj any updates here?

[fahslaj]

@amorscher sorry for the delay. I think the solution you have suggested of throwing a ValidationError in the case above makes sense. If you could please make that change and update this branch from main then we can get this merged as soon as CI passes. Thanks!

[amorscher]

@amorscher sorry for the delay. I think the solution you have suggested of throwing a ValidationError in the case above makes sense. If you could please make that change and update this branch from main then we can get this merged as soon as CI passes. Thanks!

OK should be done!

[fahslaj]

Thank you @amorscher ! Your contributions are greatly appreciated 😄