fix(rome_cli): correctly compute file to handle in rome ci
#3453
Conversation
✅ Deploy Preview for rometools canceled.
|
56c62e3 to
e65a86d
Compare
1ae05be to
e25f80b
Compare
There was a problem hiding this comment.
Since the traversal behavior is hard to get right perhaps we should add more test cases to verify that it works correctly in various edge cases (check that it's visiting the correct files, but also importantly not visiting the files that are not contained in the base path)
| @@ -161,8 +161,21 @@ impl<'scope> TraversalScope<'scope> for MemoryTraversalScope<'scope> { | |||
| { | |||
| let files = &self.fs.files.0.read(); | |||
| for path in files.keys() { | |||
| if path.strip_prefix(&base).is_ok() { | |||
| let should_process_file = if base.starts_with(".") || base.starts_with("./") { | |||
There was a problem hiding this comment.
I think the Path type has a set of is_relative / is_absolute methods to detect these cases
There was a problem hiding this comment.
That's not the case we are trying to get right. If we use is_relative, the paths test.js and ./ will both yield true in both cases. If we use is_absolute, it would give us false for both paths.
I updated the comment, to explain what we are doing.
e2b8f3f to
fdc1428
Compare
I added another test to see if that works. |
8f82c48 to
1242b88
Compare
1242b88 to
1904068
Compare
1904068 to
0b74d0b
Compare
Summary
There was an issue in the
rome cicommand, but we couldn't see it because theMemoryFileSystembehaves differently when handling the files, compared toOsFileSystem. By default, it was accepting any kind of file and it was using thestrip_prefixfunction. This results in two issues:rome format .(the dot)This PR tries to change that behaviour.
Test Plan
can_handlefunction, the testci_does_not_run_linterwas failing, because it was wrongly computing when a file is ignored. That's fixed in thetraversal.rs