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

Jump to bottom

[cli] Provide better logs related to dir structure when running tests #4213

Merged
merged 2 commits into from Jan 4, 2022
Merged

[cli] Provide better logs related to dir structure when running tests #4213

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87ec2be
add some better logs when creating lib defs
Brianzchen Dec 30, 2021
e1874e0
add explanation comments on ds store
Brianzchen Jan 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 8 additions & 4 deletions cli/src/lib/libDefs.js
View file
Open in desktop
Expand Up @@ -168,6 +168,8 @@ export async function getLibDefs(defsDir: string): Promise<Array<LibDef>> {
const defsDirItems = await fs.readdir(defsDir);
await P.all(
defsDirItems.map(async item => {
if (item === '.DS_Store') return;
Copy link
Member

Choose a reason for hiding this comment

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

Could we instead get rid of the DS_Store files and add a test to make sure they don't come back?

Copy link
Member Author

Choose a reason for hiding this comment

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

How do we do that? Mine randomly appeared because I opened my finder, it's not like you can gitignore it because the issue is when we run it locally and because it creates a DS_Store in every single dir means that when cleaning up I was trying to go into every definitions/npm/@scope to delete this.

Unless you're saying when we run this loop we should delete it instead of returning? Not sure which approach is best here, I imagine deleting isn't good because DS_Store is used to retain folder positioning if someone does open the repo in their finder which has some usage.

Copy link
Member

Choose a reason for hiding this comment

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

Right yeah makes sense, I see how it can be an issue when running it locally. 👍🏼

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I can see how this code makes no sense when reading this later. I'll add a comment to describe why this issue needs to be exited before I merge


const itemPath = path.join(defsDir, item);
const itemStat = await fs.stat(itemPath);
if (itemStat.isDirectory()) {
Expand All @@ -177,13 +179,15 @@ export async function getLibDefs(defsDir: string): Promise<Array<LibDef>> {
const defsDirItems = await fs.readdir(itemPath);
await P.all(
defsDirItems.map(async item => {
if (item === '.DS_Store') return;

const itemPath = path.join(defsDir, scope, item);
const itemStat = await fs.stat(itemPath);
if (itemStat.isDirectory()) {
// itemPath is a lib dir
await addLibDefs(itemPath, libDefs);
} else {
const error = `Expected only directories in the 'definitions/npm/@<scope>' directory!`;
const error = `Expected only directories in the 'definitions/npm/@<scope>' directory! Please remove or change ${itemPath}`;
throw new ValidationError(error);
}
}),
Expand All @@ -193,7 +197,7 @@ export async function getLibDefs(defsDir: string): Promise<Array<LibDef>> {
await addLibDefs(itemPath, libDefs);
}
} else {
const error = `Expected only directories in the 'definitions/npm' directory!`;
const error = `Expected only directories in the 'definitions/npm' directory! Please remove or change ${itemPath}`;
throw new ValidationError(error);
}
}),
Expand Down Expand Up @@ -246,7 +250,7 @@ async function parseLibDefsFromPkgDir(
}

if (flowDirs.length === 0) {
throw new ValidationError(`No libdef files found for ${pkgDirPath}!`);
throw new ValidationError(`No libdef files found in ${pkgDirPath}!`);
}

const libDefs = [];
Expand Down Expand Up @@ -290,7 +294,7 @@ async function parseLibDefsFromPkgDir(
if (libDefFilePath == null) {
libDefFilePath = path.join(flowDirPath, libDefFileName);
if (pkgName !== 'ERROR') {
const error = 'No libdef file found!';
const error = `No libdef file found in ${flowDirPath}`;
throw new ValidationError(error);
}
return;
Expand Down