[GithubGistLastCommit] GitHub gist last commit

[PaulaBarszcz]

Related issue: #7884

Locally looks like this (updated with the new path, { base: 'github-gist/last-commit', pattern: ':gistId' }):

Print screens of example gists generated today (3rd of August 2022):
https://api.github.com/gists/8710649
gistId: 8710649
"updated_at": "2022-08-03T02:58:04Z",

https://api.github.com/gists/870071abadfd66a28bf539677332f12b
gistId: 870071abadfd66a28bf539677332f12b
"updated_at": "2020-10-01T13:50:24Z",

https://api.github.com/gists/7e188c35fd5ca754c970e3a1caf045ef
gistId: 7e188c35fd5ca754c970e3a1caf045ef
"updated_at": "2020-07-27T17:28:45Z",

https://api.github.com/gists/871064
gistId: 871064
"updated_at": "2015-09-25T05:28:06Z",

https://api.github.com/gists/55555555555555
gistId: 55555555555555
"message": "Not Found",

---

I confirmed locally that it still works after the changes following the review:

[shields-ci]

	Messages
📖	✨ Thanks for your contribution to Shields, @PaulaBarszcz!

Generated by 🚫 dangerJS against 62ece81

[PaulaBarszcz]

The github-gist-last-commit service development is done- but I have issues with tests.

If I add .json extension to the call’s url, the call fails.
If I don’t add it, I receive an SVG file in a response.

This is the error that I am currently getting while running the test suite:

message.txt

[calebcartwright]

@PaulaBarszcz - the one uncommented test you have works fine with when the .json extension is added (the last commit in gist (ancient) test). The issues you mentioned offline with the others is probably because there is an unnecessary leading /gists in them. The way our createServiceTester works is that it handles determining and incorporating the base part of the badge route (the part you've currently got set to github/gists in the service classes) automatically, so the only parts that are necessary to specify in the test route are the paramters

[PaulaBarszcz]

@PaulaBarszcz - the one uncommented test you have works fine with when the .json extension is added (the last commit in gist (ancient) test). The issues you mentioned offline with the others is probably because there is an unnecessary leading /gists in them. The way our createServiceTester works is that it handles determining and incorporating the base part of the badge route (the part you've currently got set to github/gists in the service classes) automatically, so the only parts that are necessary to specify in the test route are the paramters

Thanks, the fixed tests are pushed.

Now I'm trying to figure out where can I check the available names for the test suites to put in the PR's title.
In this PR should I put [GitHub], [GitHubActivity], [GitHubCommitActivity] or something else?

I updated the PR's description.
The PR is ready to be reviewed and all comments are welcome ;)

[calebcartwright]

Thank you! This is looking pretty good, few minor things left inline for your review

[calebcartwright]

I hate to mention this since I'm sure you had fun battling nock 😄 but I think we should drop this test.

We mostly use our service tests as an integration and e2e style test suite to both give us some high level assurances about the service and routes, but particularly the touch points with the hundreds of backend APIs we integrate with (and which can change without notice).

In cases where we have service class with non trivial render or transform functions we will add more standard unit tests to validate those, but we typically refrain from doing so when those are simple functions or just using some of our standard helpers like this one is (those helpers are extensively tested elsewhere).

[PaulaBarszcz]

I hate to mention this since I'm sure you had fun battling nock 😄 but I think we should drop this test.

Haha, a little bit :D

Sure, removed this test :)

[PaulaBarszcz]

Since we're changing the route to be github-gist/last-commit,
should we also update the file names?

Currently we have:

[PaulaBarszcz]

Implementation of the changes is done, and this PR is ready for a re-review :)

[calebcartwright]

Now I'm trying to figure out where can I check the available names for the test suites to put in the PR's title.
In this PR should I put [GitHub], [GitHubActivity], [GitHubCommitActivity] or something else?

You typically only want to run the tests relevant for the code being added or modified. The more generic/higher level the filter the more tests get executed, and the greater the potential for failures/brittleness in tests that weren't actually related to proposed changes. In this case we'd actually want to use GithubGistLastCommit to run the tests here instead of the tests for the repo commit badge.

I've updated the title and re-executed the tests accordingly

[calebcartwright]

should we also update the file names?

No, these are fine as-is 👍

[PaulaBarszcz]

should we also update the file names?

No, these are fine as-is 👍

Cool, thanks.

Now I'm trying to figure out where can I check the available names for the test suites to put in the PR's title.
In this PR should I put [GitHub], [GitHubActivity], [GitHubCommitActivity] or something else?

You typically only want to run the tests relevant for the code being added or modified. The more generic/higher level the filter the more tests get executed, and the greater the potential for failures/brittleness in tests that weren't actually related to proposed changes. In this case we'd actually want to use GithubGistLastCommit to run the tests here instead of the tests for the repo commit badge.

I've updated the title and re-executed the tests accordingly

Great, thank you.

Should anything else be changed/added in this PR? :)

[calebcartwright]

Nope all set. I was just waiting for feedback from others offline relative to the route pattern for github gist badges going forward but we're good to go!