Better output for "prettier" test

[rdwebdesign]

By submitting this pull request, I confirm the following:

• I have read and understood the contributors guide, as well as this entire template.
• I have made only one major change in my proposed changes.
• I have commented my proposed changes within the code.
• I have tested my proposed changes.
• I am willing to help maintain this change if there are issues with it later.
• I give this submission freely and claim no ownership.
• It is compatible with the EUPL 1.2 license
• I have squashed any insignificant commits. (git rebase)
• I have Signed Off all commits. (git commit --signoff)

---

What does this PR aim to accomplish?:

Currently, the "prettier" test output only shows the name of the file where an error was found. This makes it difficult to correct the error.

How does this PR accomplish the above?:

This PR uses git diff to create more detailed output for the test.
The new output adds the errors found and the lines that should be modified.

What documentation changes (if any) are needed to support this PR?:

none

[yubiuser]

I tried it locally and it works. However, the diff gets only printed if you have committed your changes already. Otherwise npm test will automatically, silently modify your code without a diff output.

Sample output

chrko@ThinkPad-X230:~/Software/pihole_repositories/AdminLTE$ git commit -a -m "Test"
[pr2014 f3b23960] Test
 1 file changed, 1 insertion(+), 1 deletion(-)
chrko@ThinkPad-X230:~/Software/pihole_repositories/AdminLTE$ npm test

> AdminLTE@1.0.0 test
> npm run prettier:fix && git diff --color=always --exit-code && npm run xo


> AdminLTE@1.0.0 prettier:fix
> prettier --write "style/*.css" "style/themes/*.css" "scripts/pi-hole/**/*.js"

style/pi-hole.css 165ms
style/themes/default-auto.css 9ms
style/themes/default-dark.css 146ms
style/themes/default-darker.css 1503ms
style/themes/default-light.css 41ms
style/themes/lcars.css 242ms
scripts/pi-hole/js/auditlog.js 108ms
scripts/pi-hole/js/customcname.js 64ms
scripts/pi-hole/js/customdns.js 53ms
scripts/pi-hole/js/db_graph.js 106ms
scripts/pi-hole/js/db_lists.js 65ms
scripts/pi-hole/js/db_queries.js 101ms
scripts/pi-hole/js/debug.js 26ms
scripts/pi-hole/js/footer.js 82ms
scripts/pi-hole/js/gravity.js 14ms
scripts/pi-hole/js/groups-adlists.js 92ms
scripts/pi-hole/js/groups-clients.js 89ms
scripts/pi-hole/js/groups-domains.js 115ms
scripts/pi-hole/js/groups.js 88ms
scripts/pi-hole/js/index.js 267ms
scripts/pi-hole/js/ip-address-sorting.js 27ms
scripts/pi-hole/js/messages.js 48ms
scripts/pi-hole/js/network.js 67ms
scripts/pi-hole/js/queries.js 93ms
scripts/pi-hole/js/queryads.js 20ms
scripts/pi-hole/js/settings.js 84ms
scripts/pi-hole/js/taillog-FTL.js 14ms
scripts/pi-hole/js/taillog.js 18ms
scripts/pi-hole/js/utils.js 46ms
diff --git a/scripts/pi-hole/js/auditlog.js b/scripts/pi-hole/js/auditlog.js
index 1c8be5c2..0374d6de 100644
--- a/scripts/pi-hole/js/auditlog.js
+++ b/scripts/pi-hole/js/auditlog.js
@@ -19,7 +19,7 @@ function updateTopLists() {
     // Clear tables before filling them with data
     $("#domain-frequency td").parent().remove();
     $("#ad-frequency td").parent().remove();
-         var domaintable = $("#domain-frequency").find("tbody:last");
+    var domaintable = $("#domain-frequency").find("tbody:last");
     var adtable = $("#ad-frequency").find("tbody:last");
     var url, domain;
     for (domain in data.top_queries) {

[rdwebdesign]

Is this test used only after a PR or it's used in any other context?

[yubiuser]

I run npm test locally after I made commits before I push them to github. This way I can see if the test would fail before everyone can see my bad code styling publicly.

[rdwebdesign]

OK.
I can improve this.

[dschaper]

Set npm test as a pre-commit hook and then you won't even commit code that fails the tests..

[yubiuser]

Set npm test as a pre-commit hook and then you won't even commit code that fails the tests..

But then I would not learn anything as there is not prettier output ;-)

[rdwebdesign]

I will revert test and create a new testpr to run when testing a PR.

[rdwebdesign]

@yubiuser,

Is this working for you?

[DL6ER]

I suggest not adding it as a pre-commit but rather pre-push hook as this will limit it to run before pushing anything instead of after every commit. The machine I run my Pi-hole on is an old HP microserver with two cores and, hence, more powerful than a Raspberry Pi. However, prettier still needs like 3 seconds for checking the darker theme CSS because it is just such a huge file. Plus time it needs to check the other files. Not something I'd like to wait for several times a day. In my typical workflow, this can quickly accumulate to a minute or two of just having to wait for things.

[rdwebdesign]

@DL6ER

My intention was not to create any hooks. I just tried to improve the current test executed after commit a PR.

This PR just replaced the old test (inside test.yml) with a new test (called testpr) to be automatically used by the Github action.

Of course, anyone can use the test scripts locally, but this is another story.

[DL6ER]

I wonder if we should make it even more obvious by pushing the diff as a comment to the PR instead of having it "hidden" in the CI logs. And I agree that your making it at least visible at all is definitely already an improvement over the current simple failed status (with file but no complaint mentioned).

[PromoFaux]

I think as a comment works for me. The only reason I didn't progress on my PR to autocommit the changes was that I felt uneasy with giving a PAT perms to push to the repo (not fully explored as to whether someone could abuse this with a malicious PR)

Also might cause issues with inexperienced submitters not realising they need to pull again before making more changes. (Though I guess that could be worked around with a PR comment, too.)

[PromoFaux]

Having played about with this a lot (sorry for all the force pushes.. also I'm not sorry!) I am pleased with the results.

[pralor-bot]

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-ftl-v5-12-web-v5-9-and-core-v5-7-released/51795/1