lavocatt commented on code in PR #154:
URL: https://github.com/apache/artemis-console/pull/154#discussion_r2664911361
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ const testService = new (artemisService.constructor as any)()
Review Comment:
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.
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ 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(null as any)
Review Comment:
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.
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ 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(null as any)
+
+ testService.initialize()
+
+ const brokerObjectName = await testService.getBrokerObjectName()
+
+ expect(brokerObjectName).toBe('')
+ })
+
+ test("initialize should handle search errors gracefully", async () => {
Review Comment:
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.
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ 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(null as any)
+
+ testService.initialize()
+
+ const brokerObjectName = await testService.getBrokerObjectName()
+
+ expect(brokerObjectName).toBe('')
+ })
+
+ test("initialize should handle search errors gracefully", 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').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'
+ ])
Review Comment:
probably use more than two, to make sure it's not just a flip of a coin.
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ 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(null as any)
+
+ testService.initialize()
+
+ const brokerObjectName = await testService.getBrokerObjectName()
+
+ expect(brokerObjectName).toBe('')
+ })
+
+ test("initialize should handle search errors gracefully", 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').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'
+ ])
+
+ 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 () => {
+ 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'])
+
+ // 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('')
+ })
Review Comment:
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?
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ 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(null as any)
+
+ testService.initialize()
+
+ const brokerObjectName = await testService.getBrokerObjectName()
+
+ expect(brokerObjectName).toBe('')
+ })
+
+ test("initialize should handle search errors gracefully", 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').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'
+ ])
+
+ 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 () => {
+ 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'])
Review Comment:
You can make use of spyes to make sure the jest is actually called by every
initialization methods.
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
Review Comment:
Probably a better name for the test would be `initialize should return empty
string when no brokers found`
##########
artemis-console-extension/artemis-extension/packages/artemis-console-plugin/src/artemis-service.test.ts:
##########
@@ -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 () => {
+ 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(null as any)
+
+ testService.initialize()
+
+ const brokerObjectName = await testService.getBrokerObjectName()
+
+ expect(brokerObjectName).toBe('')
+ })
+
+ test("initialize should handle search errors gracefully", 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').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'
+ ])
+
+ 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 () => {
Review Comment:
Would this be a better name `initialize should use custom JMX domain from
config in search string`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
