Skip to content

Conversation

Beytoven
Copy link
Contributor

This check would we need to be updated every time an audit is added or removed. Could become a nuisance in time.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this works for me too though, you already know I don't think our csv automated testing is vital to Lighthouse's success 😉

@@ -101,7 +101,6 @@ describe('ReportGenerator', () => {
fs.writeFileSync(path, csvOutput);

const lines = csvOutput.split('\n');
expect(lines).toHaveLength(145);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can I sell you on expect(lines.length).toBeGreaterThan(100) or some such thing that ensures a bunch of audits made it through? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sold. As long as we don't have to update it all the time :)

@Beytoven Beytoven merged commit 0bb7117 into master Apr 30, 2020
@Beytoven Beytoven deleted the csv-test-length branch April 30, 2020 22:04
@connorjclark
Copy link
Collaborator

fyi @patrickhulce @Beytoven the title here should have been tests(csv): ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants