CHANGE @W-17311830@ FlowTest no longer makes log files #137
Conversation
| "@types/node": "^20.0.0", | ||
| "@types/semver": "^7.5.8", | ||
| "@types/tmp": "^0.2.6", | ||
| "fast-glob": "^3.3.2", |
There was a problem hiding this comment.
This shouldn't be a dependency if it is only used in test. It should be a devDepenency at best. But is there an alternative instead of adding more dependencies to our project?
There was a problem hiding this comment.
Huh, I was positive I had it as a devDependency already. Guess not.
I'll see if there's an alternative, but the problem is that the logfile's name is randomly generated, so we don't already know where it is.
I'll see what the other options are to programmatically find a file.
| it('Generates no log file', async () => { | ||
| const flowLogFiles = await fg.glob('./**/.flowtest_log_*.log'); | ||
| expect(flowLogFiles).toHaveLength(0); | ||
| }); |
There was a problem hiding this comment.
Seems heavy handed to pull in all of fast-glob just too look up if a .log file was generated somewhere underneath the current working directory. I wonder if there is some other node built in tool that we could use instead. If not then I wonder if the test is even needed or worth it.
There was a problem hiding this comment.
The test is worth it IMO because it locks in the behavior of not generating the log file.
| it('Generates no log file', async () => { | ||
| const logFileMatcher = /\.flowtest_log_.+\.log/; | ||
| const logFiles = (await fs.readdir('.')).filter(f => f.match(logFileMatcher)); | ||
| expect(logFiles).toHaveLength(0); | ||
| }); |
There was a problem hiding this comment.
This looks good now. Thanks for removing the glob dependency.
jfeingold35
force-pushed
the
d/W-17311830
branch
from
9791985 to
9591450
Compare
No description provided.