playwright: add support for test file prefixes in JSON reports#1250
playwright: add support for test file prefixes in JSON reports#1250
Conversation
This comment has been minimized.
This comment has been minimized.
a43bf62 to
21a652c
Compare
21a652c to
d4fa6e4
Compare
d4fa6e4 to
c3624f6
Compare
c598d6f to
a392c86
Compare
|
No rush at all, but can you please review this PR? What do you think about this approach? Do you foresee potential risks regarding my change? |
| for event in self._parse_suites(test_file, s, []): | ||
| yield event | ||
|
|
||
| def _compute_test_prefix(self, report: Dict) -> str: |
There was a problem hiding this comment.
I think the approach makes sense, but "test_prefix" might not be the best name.
We might be able to come up with something clearer.
There was a problem hiding this comment.
Thanks for your suggestion. I renamed some method names 👍
| if test_file.startswith(test_prefix): | ||
| return test_file | ||
|
|
||
| return Path(test_prefix, test_file).as_posix() |
There was a problem hiding this comment.
It might be better to use the method in testpath.py for this.
There was a problem hiding this comment.
Good suggestion. I extracted some test path logic and move them to the testpath.py.
|
|
||
| base_dir = Path(config_file).parent | ||
| try: | ||
| test_prefix = Path(root_dir).relative_to(base_dir).as_posix() |
There was a problem hiding this comment.
Is there a reason you didn’t use rootDir + title?
There was a problem hiding this comment.
Yes. We can get the full path by using rootDir + title, but we cannot get the relative path from the project/repo root using only those.
As a workaround to get the project/repo root, I'm comparing the configFile against rootDir.
@Konboi suggested me the following approach yesterday, this is definitely a valid approach. In this PR, I explored the another approach, which is automatically detecting test paths.
The basic idea is computing the relative directory by comparing the configFile against rootDir as follows: