Skip to content
Open
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
25 changes: 25 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
name: "Test"

on: [push, pull_request]

jobs:
build:

runs-on: ubuntu-latest

strategy:
matrix:
node-version: [22.17.1]

steps:
- uses: actions/checkout@v4
- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node-version }}
- name: install
run: yarn install
working-directory: ./artemis-console-extension/artemis-extension/packages/artemis-console-plugin
- name: test
run: yarn test
working-directory: ./artemis-console-extension/artemis-extension/packages/artemis-console-plugin
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { beforeAll, describe, expect, test } from "@jest/globals"
import { beforeAll, beforeEach, describe, expect, jest, test } from "@jest/globals"
import { artemisService, parseMBeanName } from "./artemis-service";
import { SortDirection } from './table/ArtemisTable'
import { userService } from '@hawtio/react'
import { jolokiaService, userService } from '@hawtio/react'
import { configManager } from './config-manager'

beforeAll(async () => {
// needed to determine Jolokia URL
Expand Down Expand Up @@ -45,9 +46,141 @@ describe("Artemis Service basic tests", () => {
const mbean = "org.apache.activemq.artemis:broker=\"0.0.0.0:61616\",component=acceptors,filter=\"x,y,z=a\",name=amqp"
const parsed = parseMBeanName(mbean)
expect(parsed.domain).toEqual("org.apache.activemq.artemis")
expect(parsed.properties["broker"]).toEqual("\"0.0.0.0:61616\"")
expect(parsed.properties["filter"]).toEqual("\"x,y,z=a\"")
expect(parsed.properties["broker"]).toEqual("0.0.0.0:61616")
expect(parsed.properties["filter"]).toEqual("x,y,z=a")
expect(parsed.properties["name"]).toEqual("amqp")
})

})

/**
* Tests for the initialize method in artemis-service.ts
*/
describe("ArtemisService.initialize()", () => {

beforeEach(() => {
// Clear any mocks before each test
jest.clearAllMocks()
})

test("initialize should set up brokerObjectName promise", async () => {
// Create a new instance to test initialization
const testService = new (artemisService.constructor as any)()

// Mock the config manager and jolokia service
const mockConfig = { jmx: { domain: 'org.apache.activemq.artemis' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockResolvedValue(['org.apache.activemq.artemis:broker=test'])

// Call initialize
testService.initialize()

// Get the broker object name (which should now be initialized)
const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('org.apache.activemq.artemis:broker=test')
expect(configManager.getArtemisconfig).toHaveBeenCalled()
expect(jolokiaService.search).toHaveBeenCalledWith('org.apache.activemq.artemis:broker=*')
})

test("initialize should handle empty search results", async () => {
const testService = new (artemisService.constructor as any)()

const mockConfig = { jmx: { domain: 'org.apache.activemq.artemis' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockResolvedValue([])

testService.initialize()

const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('')
})

test("initialize should handle null search results", async () => {
Copy link

Choose a reason for hiding this comment

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

Probably a better name for the test would be initialize should return empty string when no brokers found

const testService = new (artemisService.constructor as any)()
Copy link

Choose a reason for hiding this comment

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

The instanciation of the object is weird to me, i don't really understand why we're not doing
const testService = new ArtemisService, the only thing that's missing to do that is to export the ArtemisService class so that it can be imported in the test case.


const mockConfig = { jmx: { domain: 'org.apache.activemq.artemis' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockResolvedValue(null as any)
Copy link

Choose a reason for hiding this comment

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

I'm not found of null as any, it hides the fact that the correct type for the answer should be a string[]. Surely passing an empty array would be better.


testService.initialize()

const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('')
})

test("initialize should handle search errors gracefully", async () => {
Copy link

Choose a reason for hiding this comment

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

A better name would be initialize should catch errors and return empty string but as we discussed, it would probably be preferable to fail the initialization in such case.

const testService = new (artemisService.constructor as any)()

const mockConfig = { jmx: { domain: 'org.apache.activemq.artemis' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockRejectedValue(new Error('Connection failed'))

testService.initialize()

const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('')
})

test("initialize should use first broker when multiple brokers found", async () => {
const testService = new (artemisService.constructor as any)()

const mockConfig = { jmx: { domain: 'org.apache.activemq.artemis' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockResolvedValue([
'org.apache.activemq.artemis:broker=broker1',
'org.apache.activemq.artemis:broker=broker2'
])
Copy link

Choose a reason for hiding this comment

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

probably use more than two, to make sure it's not just a flip of a coin.


testService.initialize()

const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('org.apache.activemq.artemis:broker=broker1')
})

test("initialize should handle custom JMX domain from config", async () => {
Copy link

Choose a reason for hiding this comment

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

Would this be a better name initialize should use custom JMX domain from config in search string?

const testService = new (artemisService.constructor as any)()

const mockConfig = { jmx: { domain: 'custom.domain' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockResolvedValue(['custom.domain:broker=test'])

testService.initialize()

await testService.getBrokerObjectName()

expect(jolokiaService.search).toHaveBeenCalledWith('custom.domain:broker=*')
})

test("initialize can be called multiple times safely", async () => {
const testService = new (artemisService.constructor as any)()

const mockConfig = { jmx: { domain: 'org.apache.activemq.artemis' } }
jest.spyOn(configManager, 'getArtemisconfig').mockResolvedValue(mockConfig)
jest.spyOn(jolokiaService, 'search').mockResolvedValue(['org.apache.activemq.artemis:broker=test'])
Comment on lines +163 to +164
Copy link

Choose a reason for hiding this comment

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

You can make use of spyes to make sure the jest is actually called by every initialization methods.


// Call initialize multiple times
testService.initialize()
testService.initialize()
testService.initialize()

const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('org.apache.activemq.artemis:broker=test')
// Config should be fetched multiple times (once per initialize call)
expect(configManager.getArtemisconfig).toHaveBeenCalled()
})

test("getBrokerObjectName should return empty string before initialization", async () => {
const testService = new (artemisService.constructor as any)()

// Don't call initialize
const brokerObjectName = await testService.getBrokerObjectName()

expect(brokerObjectName).toBe('')
})
Comment on lines +178 to +185
Copy link

Choose a reason for hiding this comment

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

If the service returns an empty string before initialization, then I'd advocate even more for throwing an exception when receiving garbage data during the initialization phase.
Another question is, what happens if I attempt to read some data with an uninitialized artemisService, do I get an error?

})