core(runner): save lhr on -A #11509
core(runner): save lhr on -A #11509
Conversation
| @@ -156,6 +156,12 @@ class Runner { | |||
| // LHR has now been localized. | |||
| const lhr = /** @type {LH.Result} */ (i18nLhr); | |||
|
|
|||
| // Save lhr to ./latest-run, but only if -A is used. | |||
| if (settings.auditMode) { | |||
| const path = Runner._getDataSavePath(settings); | |||
There was a problem hiding this comment.
I noticed an lhr file will linger if you do -G ... I nearly wrote some code to delete the latest run folder before running, but that seems dangerous.
There was a problem hiding this comment.
I nearly wrote some code to delete the latest run folder before running, but that seems dangerous.
thank goodness you didn't, that would have been horrifying to me and the important examples I keep in that folder due to its gitignore properties 😅
(bad idea I know)
connorjclark
changed the title
| @@ -0,0 +1,140 @@ | |||
| { | |||
There was a problem hiding this comment.
nit: Is lhr considered an artifact? If not, does it make sense to have these under a folder named "artifacts"? If it's too destructive to rename this folder I'm totally fine leaving it as well.
There was a problem hiding this comment.
This doesn't bother me, and I'd prefer to avoid the churn.
There was a problem hiding this comment.
Looks like this breaks our travis assertion that no source files change by running the tests (something we should add to github assertion suite 👍 ).
Actually looking at the code now, I'm a little confused why are these checked in at all. It doesn't look like they're normalized, and we never check equality (or even their existence?). Seems like an accidental add with git -A.
There was a problem hiding this comment.
something we should add to github assertion suite 👍
soooo we already do this and now I'm confused why github passes but travis doesn't... figured it out we don't run it after the unit tests!
There was a problem hiding this comment.
figured it out we don't run it after the unit tests!
😢 😢
Should we fix that and revert these lhrs (unless there was a non-apparent reason to add them)? Future tests that might need an lhr with these artifacts can always generate them (and normalize them, etc) when needed.
| @@ -156,6 +156,12 @@ class Runner { | |||
| // LHR has now been localized. | |||
| const lhr = /** @type {LH.Result} */ (i18nLhr); | |||
|
|
|||
| // Save lhr to ./latest-run, but only if -A is used. | |||
There was a problem hiding this comment.
Add something like this to comment on line 54?
I actually think we should avoid mentioning in the readme, because we don't want people to get the idea that this is how you get a LHR. I think it'd be more confusing than anything. this feature is really only meant for contributors imo. |
Contributors read readmes, too :) It seems as worth documenting as anything else in that screenshot. |
No description provided.