Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include the font/integration/unit-test folders in the LGTM report #13772

Merged
merged 1 commit into from
Jul 22, 2021

Conversation

Snuffleupagus
Copy link
Collaborator

By default all files/folders classified as test-code are excluded in the LGTM report, however I do believe that it makes sense to include at least the font/integration/unit-test folders.
It seems pretty obvious that you want to avoid accidentally introducing any simple logical errors in the tests (and not only in the main code), to ensure that e.g. the unit-tests actually test the desired thing.

Hopefully I've interpreted the information at https://lgtm.com/help/lgtm/customizing-file-classification correctly, however note that we may need to land this patch to actually see any difference in the results.

By default all files/folders classified as test-code are excluded in the LGTM report, however I do believe that it makes sense to include at least the `font`/`integration`/`unit`-test folders.
It seems pretty obvious that you want to avoid *accidentally* introducing any simple logical errors in the tests (and not only in the main code), to ensure that e.g. the unit-tests actually test the desired thing.

Hopefully I've interpreted the information at https://lgtm.com/help/lgtm/customizing-file-classification correctly, however note that we may need to land this patch to actually see any difference in the results.
@Snuffleupagus
Copy link
Collaborator Author

Snuffleupagus commented Jul 22, 2021

I'm going to make an exception and land this without waiting for review, since:

  • We cannot tell if this actually works as intended, without landing the patch.
  • The patch is very small and simple, and only applies to the LGTM-integration.
  • Finally, and most importantly, none of this code applies to the built PDF.js library and cannot possibly affect rendering, the viewer, or any other even remotely critical functionality.

@Snuffleupagus Snuffleupagus merged commit ab7b577 into mozilla:master Jul 22, 2021
@Snuffleupagus Snuffleupagus deleted the lgtm-exclude-tests branch July 22, 2021 07:35
@Snuffleupagus Snuffleupagus restored the lgtm-exclude-tests branch July 22, 2021 07:35
@Snuffleupagus Snuffleupagus deleted the lgtm-exclude-tests branch July 22, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant