Skip to content

feat: add new badges for new [ansible] collection APIs #10938

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

Merged
merged 5 commits into from
Mar 13, 2025

Conversation

artis3n
Copy link
Contributor

@artis3n artis3n commented Mar 9, 2025

No description provided.

Copy link
Contributor

github-actions bot commented Mar 9, 2025

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

Generated by 🚫 dangerJS against f6643c1

@artis3n
Copy link
Contributor Author

artis3n commented Mar 9, 2025

Something I don't understand is my new tests fail locally, but the new endpoint works fine in npm start and the CI suite doesn't see a problem either. Why am I getting local failure?

npm run test:services -- --only=ansible

image

@artis3n artis3n marked this pull request as ready for review March 9, 2025 19:52
@artis3n artis3n force-pushed the ansible-collections-v2 branch from 51462f9 to 0200661 Compare March 9, 2025 19:52
@chris48s
Copy link
Member

On your tests:

You can call any shields badge URL with a .json extension to request a JSON response instead of SVG. So for example:

In the service tests we test the JSON response because it is easier to make assertions about the JSON response than the SVG response. The logic for rendering that object into a SVG image is tested separately.

At the moment you're requesting a SVG response then trying to make assertions about it as if it were a JSON object. Hence they're all failing with Error: Error parsing JSON string: Unexpected token <. So the reason the tests are failing is because you need to request the URLs with .json on the end.

@artis3n
Copy link
Contributor Author

artis3n commented Mar 10, 2025

Ahhh thank you, will update those test endpoints to use .json.

Now, why are the checks in the PR all passing, shouldn't something fail and flag that these tests are failing and need to be corrected?

Copy link
Contributor

🚀 Updated review app: https://pr-10938-badges-shields.fly.dev

@chris48s chris48s changed the title feat: add new badges for new ansible collection APIs feat: add new badges for new [ansible] collection APIs Mar 11, 2025
@chris48s
Copy link
Member

We hit live APIs under test, so a full service test run is slow and contains some tests that can be flaky. For this reason, we run all the core tests automatically but you have to opt in to running a subset of service tests:
https://github.com/badges/shields/blob/master/CONTRIBUTING.md#running-service-tests-in-pull-requests
I've updated the PR title so the ansible tests will run.

@artis3n
Copy link
Contributor Author

artis3n commented Mar 11, 2025

Thank you, apologies for missing that!

@chris48s chris48s added the service-badge New or updated service badge label Mar 11, 2025
@artis3n artis3n force-pushed the ansible-collections-v2 branch from 0a7130e to 5f20967 Compare March 11, 2025 21:36
@artis3n artis3n force-pushed the ansible-collections-v2 branch from 5f20967 to 4b41ea1 Compare March 13, 2025 17:04
Copy link
Contributor

🚀 Updated review app: https://pr-10938-badges-shields.fly.dev

@chris48s chris48s added this pull request to the merge queue Mar 13, 2025
Merged via the queue into badges:master with commit abf0a29 Mar 13, 2025
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service-badge New or updated service badge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants