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

Added basic support for [hockeyapp] #2055

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions services/hockeyapp/hockeyapp.service.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
'use strict'

const Joi = require('joi')
const { renderVersionBadge } = require('../../lib/version')
const BaseJsonService = require('../base-json')
const { NotFound } = require('../errors')

const hockeyappSchema = Joi.object({
app_versions: Joi.array().items(
Joi.object({
version: Joi.string().required(),
shortversion: Joi.string().required(),
})
),
}).required()

module.exports = class Hockeyapp extends BaseJsonService {
async fetch({ apitoken, appid }) {
const url = `https://rink.hockeyapp.net/api/2/apps/${appid}/app_versions`
const options = { headers: { 'X-HockeyAppToken': apitoken } }
return this._requestJson({
url,
schema: hockeyappSchema,
options: options,
})
}

static render({ version }) {
return renderVersionBadge({ version })
}

async handle({ apitoken, appid }) {
const json = await this.fetch({ apitoken, appid })

if (json.app_versions.length === 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could move this check to the schema instead:

app_versions: Joi.array()
  .items(
    Joi.Object({ ... })
  )
  .min(1),
...

throw new NotFound()
}

const latestVersionObject = json.app_versions[0]

let version

// need this check because hockeyapp handles build numbers differntly for iOS and Android
if (
!latestVersionObject.version.includes(latestVersionObject.shortversion)
) {
version = `${latestVersionObject.shortversion}.${
latestVersionObject.version
}`
} else {
version = latestVersionObject.version
}

return this.constructor.render({ version })
}

// Metadata
static get defaultBadgeData() {
return { label: 'hockeyapp' }
}

static get category() {
return 'version'
}

static get url() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this signature should now be static get route()

return {
base: 'hockeyapp/v',
format: '(.+)/(.+)',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can switch this to use pattern now: remove the format and capture keys and replace them with:

pattern: ':apioken+/:appid+'

that's assuming those two params both need the full (.+) capture group

capture: ['apitoken', 'appid'],
}
}

static get examples() {
return [
{
exampleUrl:
'78e9ae0287ca4a07b2cbbb1338e1c71b/7e53b1f0ebe171be1b9004ff04a2f663',
urlPattern: ':read-only-api-token/:appid',
staticExample: this.render({ version: '1.0.0' }),
documentation: `Log in to HockeyApp and follow the link https://rink.hockeyapp.net/manage/auth_tokens
to generate a read-only API Token to avoid that others can e.g. manipulat your build uploads.
Optionally you can also set the scope of the token just to the app that is used by your badge.
More information about the HockeyApp API can be found here https://support.hockeyapp.net/kb/api/api-basics-and-authentication.`,
},
]
}
}
55 changes: 55 additions & 0 deletions services/hockeyapp/hockeyapp.tester.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict'

const createServiceTester = require('../create-service-tester')

const { isVPlusDottedVersionAtLeastOne } = require('../test-validators')

const t = createServiceTester()
module.exports = t

t.create('Invalid AppToken or AppId')
.get('hockeyapp/v/1234/1234.json')
.expectJSON({ name: 'hockeyapp', value: 'invalid' })

t.create('Empty app versions array')
.get('hockeyapp/v/15011995/30071996.json')
.intercept(nock =>
nock('https://rink.hockeyapp.net')
.get('/api/2/apps/30071996/app_versions')
.reply(200, {
app_versions: [],
})
)
.expectJSON({ name: 'hockeyapp', value: 'not found' })

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have at least one valid case test in the suite which hits the live API and doesn't mock the response. I realise it is unusual practice for unit tests to depend on an external service ("don't test what you don't own") but we integrate with hundreds of services. If an upstream API changes or goes away, we need a test to start failing on the daily build otherwise we won't notice it.

Copy link
Contributor Author

@TobiasRoeddiger TobiasRoeddiger Sep 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally understand that but we would have to rely on an app token that has to be obtained with a real account at https://hockeyapp.net - how do you handle this situation?

t.create('Android App')
.get('hockeyapp/v/15011995/30071996.json')
.intercept(nock =>
nock('https://rink.hockeyapp.net')
.get('/api/2/apps/30071996/app_versions')
.reply(200, {
app_versions: [
{
version: '24',
shortversion: '1.0',
},
],
})
)
.expectJSONTypes({ name: 'hockeyapp', value: isVPlusDottedVersionAtLeastOne })

t.create('iOS App')
.get('hockeyapp/v/15011995/30071996.json')
.intercept(nock =>
nock('https://rink.hockeyapp.net')
.get('/api/2/apps/30071996/app_versions')
.reply(200, {
app_versions: [
{
version: '1.0.24',
shortversion: '1.0',
},
],
})
)
.expectJSONTypes({ name: 'hockeyapp', value: isVPlusDottedVersionAtLeastOne })