Conversation
| @@ -0,0 +1,273 @@ | |||
| # Testing Philosophy & Practices for @just-in/core | |||
There was a problem hiding this comment.
I love this! I'll need to spend more time with it to wrap my head around the parts about mocking and spying and/or if those parts might need a bit more, but overall the philosophy and approach is great and I love that you put in the effort to get this document off the ground!
| } | ||
| } | ||
|
|
||
| let _globalMinLevel: string = (process.env.LOG_LEVEL as string) ?? 'DEBUG'; |
There was a problem hiding this comment.
[Non blocking] Is it correct to understand it as the default min level that will be applied if nothing is specified? In other words, it is the default value that can be used globally across an application.
| import { NO_ID } from '../data-manager.constants'; | ||
| import { createLogger } from '../../logger/logger'; | ||
|
|
||
| const Log = createLogger({ |
There was a problem hiding this comment.
It's very exciting/lovely to see we can declare logger with context to scope it. Very easy to use and understand.
| collectionName: string, | ||
| changeType: CollectionChangeType | ||
| ): void { | ||
| public removeChangeListener(collectionName: string, changeType: CollectionChangeType): void { |
There was a problem hiding this comment.
[For the body] It takes some time for me to understand why we need a custom cleanup for/before destruction, but now I can appreciate the thinking behind it.
| * @param {string} storeName - The collection/table name. | ||
| * @param {Array<{name?: string; key: unknown; unique?: boolean; partialFilterExpression?: unknown}>} indexes | ||
| */ | ||
| public async ensureIndexes( |
There was a problem hiding this comment.
Love the support of index! It will be super useful.
| throw new Error('MongoDB is the only supported DB type'); | ||
| } | ||
| await this.db.init(); | ||
| this.isInitialized = true; |
There was a problem hiding this comment.
I was wondering whether we actually need isInitialized in data-manager, instead of letting the db (mongo-manager in this case) provide that status. It seems that the status is entirely dependent on the underlying DB's status. In that case, would it be better to have a single source of truth if no additional initialization is required?
| if (collectionName === USERS) { | ||
| this.emit('userAdded', newItem); |
There was a problem hiding this comment.
Should we standardize it so that it could in theory work for any collections that need these events?
Something like
const eventCollection = [USERS];
if (collectionName in eventCollection) {
this.emit('${collectionName}-addItem', newItem);
}
| public async findItemsInCollection<T>( | ||
| collectionName: string, | ||
| criteria: Record<string, any> | ||
| criteria: Record<string, any>, |
There was a problem hiding this comment.
Update the comment to reflect Record<string, any>
| (document: { fullDocument: object; updateDescription?: object }): Promise<void>; | ||
| }; | ||
|
|
||
| export type CollectionChangeNotifier = { |
There was a problem hiding this comment.
I cannot find how we used it. If that is the case, consider remove it?
| _users.clear(); | ||
| const userDocs = | ||
| (await dm.getAllInCollection<JUser>(USERS)) || []; | ||
| const userDocs = (await dm.getAllInCollection<JUser>(USERS)) || []; |
There was a problem hiding this comment.
This is minor. It's a bit strange that we are getting what is supposed to be JUser and then still have to transform it into JUser below. Isn't the transformation already provided by the database manager (through mongo-manager)?
There was a problem hiding this comment.
I was wondering about your current thoughts on the input validation for different methods in this module. Should it be done differently? We discussed this before, and I contributed some, if not most, of them. Now that I looked at it a second time, it seems a lot. It would be great to hear your thoughts.
… environments utilizing core
3 participants
Description
Related Jira Tickets
Comments
Please ensure all items are complete before requesting a review:
anytypes without proper justification.console.logor other debugging output from the code.