Skip to content
This repository has been archived by the owner on Dec 28, 2024. It is now read-only.

LL-277 #97

Merged
merged 6 commits into from
Aug 27, 2024
Merged

LL-277 #97

merged 6 commits into from
Aug 27, 2024

Conversation

KevinLemon112
Copy link
Collaborator

@KevinLemon112 KevinLemon112 commented Aug 22, 2024

Description

In this new PR/Ticket, unit tests for the Publish and Subscribe features for the ActiveMQ notification service have been created and tested.

Subsequent # (issue): LL-277

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Steps (For Local Host):

  1. Be in the server/src/notification/publish OR server/src/notification/subscribe directories to test each unit test separately
  2. Run "npm i" if needed in each directory (as well as the root directory)
  3. Run "npx jest" in the terminal to test the unit test for the directory you are in.

Checklist (when relevant):

  • My code follows the style guidelines of this project

TypeScript
Python
Schema

  • I have included a reviewer on the pull request [Replace this text with the reviewer]
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@KevinLemon112 KevinLemon112 added the notification Notification Feature label Aug 22, 2024
@KevinLemon112 KevinLemon112 self-assigned this Aug 22, 2024
});

it('should publish a message and return the event object', async () => {
const response = await request(app).get('/publish/12345');
Copy link
Collaborator

Choose a reason for hiding this comment

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

@KevinLemon112 I think this request should be mocked. Please refer to this stack overflow article as an example of how to mock requests to an ExpressJS Server.
https://stackoverflow.com/a/72674717

@KevinLemon112 KevinLemon112 requested a review from pogi7 August 26, 2024 06:26
@Verus20
Copy link
Collaborator

Verus20 commented Aug 27, 2024

@KevinLemon112 the tests work nice job! recommendation: In the How has this been tested stage, I'd recommending adding a step to run npm i from the root directory of the server (i.e the folder before src)
image

This is because I was running into errors with jest and then running npm i from the root folder got the tests to work

@Verus20 Verus20 merged commit f93db37 into master Aug 27, 2024
5 of 7 checks passed
@Verus20 Verus20 deleted the LL-277 branch August 27, 2024 19:07
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
notification Notification Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants