Skip to content
This repository was archived by the owner on Feb 12, 2024. It is now read-only.

Less skipped tests #494

Merged
merged 3 commits into from
Sep 15, 2016
Merged

Less skipped tests #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2beac9c
fix(config): better http-api and interface-ipfs-core compliant
dignifiedquire Sep 15, 2016
0ee7215
fix(version): better http-api and interface-ipfs-core compliant
dignifiedquire Sep 15, 2016
be6f679
test(id): test id on the cli
dignifiedquire Sep 15, 2016
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
42 changes: 7 additions & 35 deletions src/cli/commands/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
'use strict'

const debug = require('debug')
const get = require('lodash.get')
const set = require('lodash.set')
const log = debug('cli:config')
log.error = debug('cli:config:error')
const utils = require('../utils')
Expand Down Expand Up @@ -43,26 +41,17 @@ module.exports = {

if (!value) {
// Get the value of a given key

if (utils.isDaemonOn()) {
return ipfs.config.get(key, (err, config) => {
if (err) {
log.error(err)
throw new Error('failed to read the config')
}

console.log(config.Value)
})
}

ipfs.config.get((err, config) => {
ipfs.config.get(key, (err, value) => {
if (err) {
log.error(err)
throw new Error('failed to read the config')
}

const value = get(config, key)
console.log(value)
if (typeof value === 'object') {
console.log(JSON.stringify(value, null, 2))
} else {
console.log(value)
}
Copy link
Member

Choose a reason for hiding this comment

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

Same here as below, we should probably try using the logger rather than console.log

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 make that in a single PR and change all of them, no?

})
} else {
// Set the new value of a given key
Expand All @@ -78,28 +67,11 @@ module.exports = {
}
}

if (utils.isDaemonOn()) {
return ipfs.config.set(key, value, (err) => {
if (err) {
log.error(err)
throw new Error('failed to save the config')
}
})
}

ipfs.config.get((err, originalConfig) => {
ipfs.config.set(key, value, (err) => {
if (err) {
log.error(err)
throw new Error('failed to read the config')
}

const updatedConfig = set(originalConfig, key, value)
ipfs.config.replace(updatedConfig, (err) => {
if (err) {
log.error(err)
throw new Error('failed to save the config')
}
})
})
}
})
Expand Down
6 changes: 1 addition & 5 deletions src/cli/commands/version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@ module.exports = {
throw err
}

if (typeof version === 'object') { // js-ipfs-api output
version = version.Version
}

console.log(`js-ipfs version: ${version}`)
console.log(`js-ipfs version: ${version.version}`)
Copy link
Member

Choose a reason for hiding this comment

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

Should probably use the logger that I've created before, instead of using console.log

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's open an issue for that, and do the conversion, if we do it at once, rather than here and there.

Copy link
Member

@victorb victorb Sep 15, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Sounds good to me. #495

})
})
}
Expand Down
64 changes: 43 additions & 21 deletions src/http-api/resources/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,10 @@ exports.getOrSet = {
handler: (request, reply) => {
const key = request.pre.args.key
const value = request.pre.args.value
const ipfs = request.server.app.ipfs

if (typeof value === 'object' && value.type === 'Buffer') {
if (typeof value === 'object' &&
value.type === 'Buffer') {
return reply({
Message: 'Invalid value type',
Code: 0
Expand All @@ -67,7 +69,7 @@ exports.getOrSet = {

if (value === undefined) {
// Get the value of a given key
return request.server.app.ipfs.config.get((err, config) => {
return ipfs.config.get((err, config) => {
if (err) {
log.error(err)
return reply({
Expand All @@ -89,9 +91,20 @@ exports.getOrSet = {
Value: value
})
})
} else {
// Set the new value of a given key
request.server.app.ipfs.config.get((err, originalConfig) => {
}

// Set the new value of a given key
ipfs.config.get((err, originalConfig) => {
if (err) {
log.error(err)
return reply({
Message: 'Failed to get config value: ' + err,
Code: 0
}).code(500)
}

const updatedConfig = set(originalConfig, key, value)
ipfs.config.replace(updatedConfig, (err) => {
if (err) {
log.error(err)
return reply({
Expand All @@ -100,28 +113,37 @@ exports.getOrSet = {
}).code(500)
}

const updatedConfig = set(originalConfig, key, value)
request.server.app.ipfs.config.replace(updatedConfig, (err) => {
if (err) {
log.error(err)
return reply({
Message: 'Failed to get config value: ' + err,
Code: 0
}).code(500)
}

return reply({
Key: key,
Value: value
})
return reply({
Key: key,
Value: value
})
})
}
})
}
}

exports.get = (request, reply) => {
return request.server.app.ipfs.config.get((err, config) => {
const ipfs = request.server.app.ipfs

ipfs.config.get((err, config) => {
if (err) {
log.error(err)
return reply({
Message: 'Failed to get config value: ' + err,
Code: 0
}).code(500)
}

return reply({
Value: config
})
})
}

exports.show = (request, reply) => {
const ipfs = request.server.app.ipfs

ipfs.config.get((err, config) => {
if (err) {
log.error(err)
return reply({
Expand Down
10 changes: 8 additions & 2 deletions src/http-api/resources/version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,17 @@ const boom = require('boom')
exports = module.exports

exports.get = (request, reply) => {
request.server.app.ipfs.version((err, version) => {
const ipfs = request.server.app.ipfs

ipfs.version((err, version) => {
if (err) {
return reply(boom.badRequest(err))
}

reply(version)
reply({
Version: version.version,
Commit: version.commit,
Repo: version.repo
})
})
}
2 changes: 1 addition & 1 deletion src/http-api/routes/config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ module.exports = (server) => {
api.route({
method: '*',
path: '/api/v0/config/show',
handler: resources.config.get
handler: resources.config.show
})

api.route({
Expand Down
4 changes: 1 addition & 3 deletions test/cli/test-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,16 +123,14 @@ describe('config', () => {
})

after((done) => {
console.log('stopping')
httpAPI.stop((err) => {
console.log('stopped')
expect(err).to.not.exist
done()
})
})

describe('get/set', () => {
it.skip('get a config key value', (done) => {
it('get a config key value', (done) => {
nexpect.spawn('node', [process.cwd() + '/src/cli/bin.js', 'config', 'Identity.PeerID'], {env})
.run((err, stdout, exitcode) => {
const expected = 'QmQ2zigjQikYnyYUSXZydNXrDRhBut2mubwJBaLXobMt3A'
Expand Down
45 changes: 13 additions & 32 deletions test/cli/test-id.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,16 @@ describe('id', () => {
env.IPFS_PATH = repoPath

describe('api offline', () => {
it.skip('get the id', (done) => {
it('get the id', (done) => {
nexpect.spawn('node', [process.cwd() + '/src/cli/bin.js', 'id'], {env})
.run((err, stdout, exitcode) => {
expect(
stdout
).to.be.eql([
'{',
' "ID": "QmQ2zigjQikYnyYUSXZydNXrDRhBut2mubwJBaLXobMt3A",',
' "PublicKey": "CAASpgIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC2SKo/HMFZeBml1AF3XijzrxrfQXdJzjePBZAbdxqKR1Mc6juRHXij6HXYPjlAk01BhF1S3Ll4Lwi0cAHhggf457sMg55UWyeGKeUv0ucgvCpBwlR5cQ020i0MgzjPWOLWq1rtvSbNcAi2ZEVn6+Q2EcHo3wUvWRtLeKz+DZSZfw2PEDC+DGPJPl7f8g7zl56YymmmzH9liZLNrzg/qidokUv5u1pdGrcpLuPNeTODk0cqKB+OUbuKj9GShYECCEjaybJDl9276oalL9ghBtSeEv20kugatTvYy590wFlJkkvyl+nPxIH0EEYMKK9XRWlu9XYnoSfboiwcv8M3SlsjAgMBAAE=",',
' "Addresses": [',
' "/ip4/127.0.0.1/tcp/9990/ws",',
' "/ip4/127.0.0.1/tcp/9999"',
' ],',
' "AgentVersion": "js-ipfs",',
' "ProtocolVersion": "9000"',
'}'
])

expect(err).to.not.exist
expect(exitcode).to.equal(0)

const id = JSON.parse(stdout.join(''))
expect(id).to.have.property('id')
expect(id).to.have.property('publicKey')
expect(id).to.have.property('addresses')
done()
})
})
Expand All @@ -55,26 +45,17 @@ describe('id', () => {
})
})

it.skip('get the id', (done) => {
it('get the id', (done) => {
nexpect.spawn('node', [process.cwd() + '/src/cli/bin.js', 'id'], {env})
.run((err, stdout, exitcode) => {
expect(
stdout
).to.be.eql([
'{',
' "ID": "QmQ2zigjQikYnyYUSXZydNXrDRhBut2mubwJBaLXobMt3A",',
' "PublicKey": "CAASpgIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQC2SKo/HMFZeBml1AF3XijzrxrfQXdJzjePBZAbdxqKR1Mc6juRHXij6HXYPjlAk01BhF1S3Ll4Lwi0cAHhggf457sMg55UWyeGKeUv0ucgvCpBwlR5cQ020i0MgzjPWOLWq1rtvSbNcAi2ZEVn6+Q2EcHo3wUvWRtLeKz+DZSZfw2PEDC+DGPJPl7f8g7zl56YymmmzH9liZLNrzg/qidokUv5u1pdGrcpLuPNeTODk0cqKB+OUbuKj9GShYECCEjaybJDl9276oalL9ghBtSeEv20kugatTvYy590wFlJkkvyl+nPxIH0EEYMKK9XRWlu9XYnoSfboiwcv8M3SlsjAgMBAAE=",',
' "Addresses": [',
' "/ip4/127.0.0.1/tcp/9990/ws",',
' "/ip4/127.0.0.1/tcp/9999"',
' ],',
' "AgentVersion": "js-ipfs",',
' "ProtocolVersion": "9000"',
'}'
])

expect(err).to.not.exist
expect(exitcode).to.equal(0)

const id = JSON.parse(stdout.join(''))
expect(id).to.have.property('id')
expect(id).to.have.property('publicKey')
expect(id).to.have.property('addresses')

done()
})
})
Expand Down
4 changes: 2 additions & 2 deletions test/cli/test-version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('version', () => {
const env = _.clone(process.env)
env.IPFS_PATH = repoPath

describe.skip('api offline', () => {
describe('api offline', () => {
it('get the version', (done) => {
nexpect.spawn('node', [process.cwd() + '/src/cli/bin.js', 'version'], {env})
.run((err, stdout, exitcode) => {
Expand All @@ -24,7 +24,7 @@ describe('version', () => {
})
})

describe.skip('api running', () => {
describe('api running', () => {
let httpAPI

before((done) => {
Expand Down
6 changes: 3 additions & 3 deletions test/http-api/inject/test-version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ module.exports = (http) => {
method: 'GET',
url: '/api/v0/version'
}, (res) => {
expect(res.result.version).to.equal(pkgversion)
expect(res.result).to.have.a.property('commit')
expect(res.result).to.have.a.property('repo')
expect(res.result).to.have.a.property('Version', pkgversion)
Copy link
Member

Choose a reason for hiding this comment

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

Should not actually check the version rather than just making sure something is there?

Copy link
Member

Choose a reason for hiding this comment

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

Suggestion would be to read version from somewhere, then assert with that, rather than hard coding the version

Copy link
Member Author

Choose a reason for hiding this comment

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

that's exactly what is happening here, pkgversion is read from package.json which is the version that js-ipfs uses as baseline when answering the version command.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, crap, you're right, as always. I'll never get used to Chai/Expect syntax...

expect(res.result).to.have.a.property('Commit')
expect(res.result).to.have.a.property('Repo')
done()
})
})
Expand Down
4 changes: 2 additions & 2 deletions test/http-api/ipfs-api/test-config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ module.exports = (ctl) => {
it('.get updatedConfig', (done) => {
ctl.config.get((err, config) => {
expect(err).not.to.exist
expect(config).to.deep.equal(updatedConfig())
expect(config).to.be.eql(updatedConfig())
done()
})
})

// This one is one stale mode till go-ipfs decides
// what to do
describe.skip('.replace', () => {
describe('.replace', () => {
it('returns error if the config is invalid', (done) => {
const filePath = 'test/test-data/badconfig'

Expand Down