Skip to content

Commit 8c38749

Browse files
committed
resolve co-pilot suggestions
1 parent 3f36abf commit 8c38749

5 files changed

Lines changed: 45 additions & 14 deletions

File tree

bin/Hubot.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const switches = [
1111
['-f', '--file HUBOT_FILE', 'Path to adapter file, e.g. "./adapters/CustomAdapter.mjs"'],
1212
['-c', '--create HUBOT_CREATE', 'Create a deployable hubot'],
1313
['-d', '--disable-httpd HUBOT_HTTPD', 'Disable the HTTP server'],
14-
['-s', '--httpdserver HUBOT_HTTPD_SERVER', 'Specify a custom HTTP server module'],
14+
['-s', '--httpdserver HUBOT_HTTPD_SERVER', 'An async factory function that receives this robot (via options) and returns or creates an HttpServerPort instance. Defaults to a factory that instantiates HttpServerPort'],
1515
['-h', '--help', 'Display the help information'],
1616
['-l', '--alias HUBOT_ALIAS', "Enable replacing the robot's name with alias"],
1717
['-n', '--name HUBOT_NAME', 'The name of the robot in chat'],

src/ports/ExpressHttpServerPort.mjs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import express from 'express'
33
import basicAuth from 'express-basic-auth'
44

55
class ExpressHttpServerPort extends HttpServerPort {
6-
constructor(robot) {
7-
super(robot)
6+
get address() {
7+
return this.robot.server.address()
88
}
99
async start(port) {
1010
const user = process.env.EXPRESS_USER
@@ -16,7 +16,7 @@ class ExpressHttpServerPort extends HttpServerPort {
1616
const app = express()
1717

1818
app.use((req, res, next) => {
19-
res.setHeader('X-Powered-By', `hubot/${encodeURI(this.name)}`)
19+
res.setHeader('X-Powered-By', `hubot/${encodeURI(this.robot.name)}`)
2020
return next()
2121
})
2222

@@ -44,10 +44,25 @@ class ExpressHttpServerPort extends HttpServerPort {
4444
}
4545
})
4646
}
47+
48+
async stop() {
49+
if (!this.robot || !this.robot.server) {
50+
return
51+
}
52+
return new Promise((resolve, reject) => {
53+
this.robot.server.close(err => {
54+
if (err) {
55+
return reject(err)
56+
}
57+
resolve()
58+
})
59+
})
60+
}
4761
}
4862

4963
export default async (robot) => {
5064
const server = new ExpressHttpServerPort(robot)
5165
const port = process.env.EXPRESS_PORT || process.env.PORT || 8080
52-
return await server.start(port)
66+
await server.start(port)
67+
return server
5368
}

src/ports/HttpServerPort.mjs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,9 @@
1-
function randomPort() {
2-
return Math.floor(Math.random() * (65535 - 1024)) + 1024
3-
}
41
class HttpServerPort {
52
constructor(robot) {
63
this.robot = robot
74
}
85
async stop() {}
96
async start(port) {
10-
if (port === 0) {
11-
port = randomPort()
12-
}
137
this.port = port
148
this.robot.server = {}
159
this.robot.router = this

test/Robot_test.mjs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import http from 'node:http'
77
import { Robot, CatchAllMessage, EnterMessage, LeaveMessage, TextMessage, TopicMessage, User, Response } from '../index.mjs'
88
import mockAdapter from './fixtures/MockAdapter.mjs'
99
import { HttpServerPort } from '../src/ports/HttpServerPort.mjs'
10-
import { ExpressHttpServerPort } from '../src/ports/ExpressHttpServerPort.mjs'
1110

1211
describe('Robot', () => {
1312
describe('#http', () => {
@@ -1047,7 +1046,30 @@ describe('Robot', () => {
10471046

10481047
describe('Non-express HTTP server', async () => {
10491048
it('should work with a non-express http server', async () => {
1050-
1049+
class HttpServer extends HttpServerPort {
1050+
async start(port) {
1051+
return new Promise((resolve, reject) => {
1052+
try {
1053+
this.robot.server = http.createServer((req, res) => {
1054+
res.writeHead(200, { 'Content-Type': 'text/plain' })
1055+
res.end('Hello World\n')
1056+
})
1057+
this.robot.router = this
1058+
this.robot.server = this.robot.server.listen(port, '127.0.0.1', () => {
1059+
this.robot.emit('listening', this.robot.server)
1060+
resolve(this.robot.server)
1061+
})
1062+
} catch (err) {
1063+
reject(err)
1064+
}
1065+
})
1066+
}
1067+
}
1068+
const httpDServerFactory = async (robot) => {
1069+
const server = new HttpServer(robot)
1070+
await server.start(process.env.PORT || 0)
1071+
return server
1072+
}
10511073
const robot = new Robot(mockAdapter, httpDServerFactory, 'TestHubot')
10521074
await robot.loadAdapter()
10531075
await robot.run()

test/contracts/HttpServer_test.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ test('HttpServerPort API', async t => {
66
await t.test('start and stop', async () => {
77
const server = new HttpServerPort({})
88
await server.start(0)
9-
assert.ok(server.address().port > 0, 'Server should be listening on a port')
9+
assert.equal(server.address().port, 0)
1010
await server.stop()
1111
})
1212

0 commit comments

Comments
 (0)