loki approve fails with "no images found to approve" when fileNameFormatter produces subfolders

[florian-milky]

loki test returns a bunch of failed tests
when I want to approve all changes with loki approve, it tells me no images found to approve
Why is that?

[techeverri]

loki test returns a bunch of failed tests
when I want to approve all changes with loki approve, it tells me no images found to **approve**

The code that prints the error is very simple and the only obvious way is that there is no valid PNGs in the output folder

loki/packages/runner/src/commands/approve/index.js

Lines 15 to 21 in 8beda16

	const files = fs.readdirSync(outputDir).filter(isPNG);
	if (!files.length) {
	die(
	'No images found to approve',
	'Run update command to generate reference files instead'
	);
	}

Why is that?

Hard to know without a way to reproduce it.

[florian-milky]

Thanks @techeverri I'm gonna try to debug it :)

[techeverri]

If you find a bug please reopen this issue or create a new one 👍

[florian-milky]

@techeverri fs.readdirSync(outputDir) returns only top folders desktop and mobile which are my configurations

/current contains desktop and mobile, images are in these subfolders

My node version is v14.13.1
loki 0.28.0

[techeverri]

How does your fileNameFormatter look like?
🔗 https://loki.js.org/configuration.html#filenameformatter

Does it happen on Windows, Linux or macOS?

[florian-milky]

How does your fileNameFormatter look like?

fileNameFormatter: ({ configurationName, kind, story }) => {
    return `${configurationName}/${kind}--${story}`;
  },

I'm on macOS.

Is loki supposed to work well with subdirectories?

I have a script that generates the approve cmd file by file so I fixed that. However I noticed it's a bit confusing because in the story filter I had to replace the -- that I get from the file name with a space.

[techeverri]

The defaultFileNameFormatter looks like this 👇

loki/packages/runner/src/commands/test/get-output-paths.js

Lines 9 to 10 in 8beda16

	const defaultFileNameFormatter = ({ configurationName, kind, story }) =>
	slugify(`${configurationName} ${kind} ${story}`, SLUGIFY_OPTIONS);

It doesn't generate subfolders and so fs.readdirSync(outputDir) works as expected with a flat structure

[techeverri]

It sounds like we need extra logic in all places that read from outputDir to support when the config has a custom fileNameFormatter

[florian-milky]

It sounds like we need extra logic in all places that read from outputDir to support when the config has a custom fileNameFormatter

Would you recommend keeping a flat structure? I don't think it's a deal breaker, you're not really supposed to browse the images manually anyway

[florian-milky]

related to #229 I would say

[d8corp]

Added a PR to fix approve command with custom fileNameFormatter (for deep file structure)

#532