-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
new badge: f-droid #1965
new badge: f-droid #1965
Conversation
Generated by 🚫 dangerJS |
services/f-droid/f-droid.tester.js
Outdated
.get('/f-droid/version/org.pacien.tincapp.json') | ||
.intercept(nock => | ||
nock('https://f-droid.org') | ||
.get('/en/packages/org.pacien.tincapp/') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are red although and I do not know why.
$ npm run test:services -- --grep F-Droid
> gh-badges@1.3.0 test:services /home/user/shields
> HANDLE_INTERNAL_ERRORS=false mocha --delay lib/service-test-runner/cli.js "--grep" "F-Droid"
Running all service tests.
No secret data found at /home/user/shields/private/secret.json (see lib/server-secrets.js)
raven@2.4.2 alert: no DSN provided, error reporting disabled
0823224940 FsTokenPersistence configured with /home/user/shields/private/github-user-tokens.json
0823224940 Server is starting up: http://lib/service-test-runner/cli.js:80/
No fallback font set.
Unable to load fallback font. Using Helvetica-Bold instead.
F-Droid
Package is found
GET /f-droid/version/org.pacien.tincapp.json has FAILED with the following response:
1)
[ GET /f-droid/version/org.pacien.tincapp.json ]
Package is not found
GET /f-droid/version/org.pacien.tincapp.json has FAILED with the following response:
2)
[ GET /f-droid/version/org.pacien.tincapp.json ]
0 passing (827ms)
2 failing
1) F-Droid
Package is found
[ GET /f-droid/version/org.pacien.tincapp.json ]:
AssertionError: expected { Object (name, value) } to deeply equal { Object (name, value) }
+ expected - actual
{
- "name": "404"
- "value": "badge not found"
+ "name": "F-Droid"
+ "value": "v1.8asdsadsa"
}
at Object.pathMatch.matchJSON (node_modules/icedfrisby/lib/pathMatch.js:138:38)
at _expect (node_modules/icedfrisby/lib/icedfrisby.js:583:10)
at IcedFrisbyNock._invokeExpects (node_modules/icedfrisby/lib/icedfrisby.js:1261:26)
at start (node_modules/icedfrisby/lib/icedfrisby.js:1244:12)
at Request.runCallback [as _callback] (node_modules/icedfrisby/lib/icedfrisby.js:1131:16)
at Request.self.callback (node_modules/request/request.js:185:22)
at Request.<anonymous> (node_modules/request/request.js:1161:10)
at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
at endReadableNT (_stream_readable.js:1056:12)
at _combinedTickCallback (internal/process/next_tick.js:138:11)
at process._tickDomainCallback (internal/process/next_tick.js:218:9)
2) F-Droid
Package is not found
[ GET /f-droid/version/org.pacien.tincapp.json ]:
AssertionError: expected { Object (name, value) } to deeply equal { Object (name, value) }
+ expected - actual
{
- "name": "404"
- "value": "badge not found"
+ "name": "F-Droid"
+ "value": "app not found"
}
at Object.pathMatch.matchJSON (node_modules/icedfrisby/lib/pathMatch.js:138:38)
at _expect (node_modules/icedfrisby/lib/icedfrisby.js:583:10)
at IcedFrisbyNock._invokeExpects (node_modules/icedfrisby/lib/icedfrisby.js:1261:26)
at start (node_modules/icedfrisby/lib/icedfrisby.js:1244:12)
at Request.runCallback [as _callback] (node_modules/icedfrisby/lib/icedfrisby.js:1131:16)
at Request.self.callback (node_modules/request/request.js:185:22)
at Request.<anonymous> (node_modules/request/request.js:1161:10)
at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
at endReadableNT (_stream_readable.js:1056:12)
at _combinedTickCallback (internal/process/next_tick.js:138:11)
at process._tickDomainCallback (internal/process/next_tick.js:218:9)
See https://f-droid.org/en/packages/org.pacien.tincapp/
and https://github.com/badges/shields/pull/1965/files#diff-63441b8c1c2f41111e85c4e116ac3804R10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved: The path is /version
and not /f-droid/version
Now, I have the problem, that the nock data is not delivered as website. The HTTP get always returns an empty website. See yourself: search for
|
Status:
|
In terms of the mock, I'm not sure what is up with those APM tests passing an array to I got your tests to pass by changing: .reply([200, manyVersions]) to: .reply(200, manyVersions) |
@paulmelnikow Thanks for the reply. I was assuming something else because of |
@PyvesB @chris48s @paulmelnikow Thanks for your replies and the help. Please review as I think, this is ready to merge. |
Looks like you need to run prettier for the test to pass. |
I forgot: The examples do not work. The tests pass but I can not use the front page. http://[::]:8080/examples/build shows
And I see this in the main page console:
I cannot find this string anywhere:
It looks like the next build is is not updated. |
The service works on http://localhost:3000/#/ but not on http://[::]:8080/ |
Problem now: I do not see the example badge. |
Problem ideintified:
is a 404 badge not found |
Solution:
The - needs to be escaped when in title. |
Problem: When a - was in the title, this lead to an invalid preview badge badges#1965 (comment) Solution: Escape the title of the badge.
this is the first untested version - get the website - parse as xml - get version info
Some services only require HTTP requests and not JSON requests. This splits up the two services.
This introduces a new error because the nock results in an empty website.
F-Droid allows downloading a 1.360.393 bytes big jar file with meta data of all the different apps. https://f-droid.org/repo/index-v1.jar This is a jar file which contains JSON.
If there were many people using the f-droid badges, caching this big file would reduce communication to external servers. It also depends on how often this file is updated. I do not think I will use this as a base for the badge. |
- improved performance - addresses badges#1965 (comment)
I think, I addressed the changes marked in the roadmap #1965 (comment). |
Thank you for addressing these! I will leave this to @PyvesB to wrap up.
I agree with you. It will be lighter to just snag the text files we need. Down the line, we could revisit this if we find many people are using these badges. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy with the current state of things. Let's get this merged! Thanks for all your work @niccokunzmann, we've all definitely learned at lot here and hopefully things will be smoother next time. 😉
Here's a summary of the follow up tasks to this PR:
- Move request method to super class and delete BaseHttp. I'm happy to tackle this one in the coming days as it's related to another piece of work I'm doing.
- Make sure all badges consistently use lower casing for labels. See Lowercase badge labels #1997.
- Make sure all services use the renderVersionBadge function consistently. See Ensure consistent usage of renderVersionBadge #2026.
- Consolidate our usage of URL encoding functions. See Consolidate URL encoding functions #2027.
- Keep an eye on GitLab and any progress on a more appropriate F-Droid API.
@PyvesB @paulmelnikow @RedSparr0w and of cause @shields-ci Thanks for the help during the pull request! |
I document the progress here as an imperfect PR and ask for review once I am confident it works.
fixes #1963