Skip to content

Conversation

kchepikava
Copy link
Contributor

No description provided.

@CLAassistant
Copy link

CLAassistant commented Jul 8, 2024

CLA assistant check
All committers have signed the CLA.

@ArtemHoruzhenko ArtemHoruzhenko changed the base branch from main to feature/RI-5902-rdi-integration July 8, 2024 07:23
},
{
provide: RdiClientStorage,
useFactory: jest.fn(() => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

let's move this factory to mocks the same way as we do everywhere

},
{
provide: RdiClientFactory,
useFactory: jest.fn(() => ({
Copy link
Collaborator

Choose a reason for hiding this comment

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

the same

message: result.message,
statusCode: HttpStatus.NOT_FOUND,
error: 'RdiNotFound',
errorCode: CustomErrorCodes.ConvAiNotFound,
Copy link
Collaborator

Choose a reason for hiding this comment

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

ConvAI error on RDI? Could you, please fix it in code.

transformations: {
status: RdiDyRunJobStatus.Success,
// data: {},
// error: ''
Copy link
Collaborator

Choose a reason for hiding this comment

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

comments?

},
{
provide: RdiPipelineAnalytics,
useValue: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

could you please move this to mocks the same way as here useFactory: mockRdiClientProvider,?


describe('dryRunJob', () => {
it('should call getOrCreate on rdiClientProvider with the correct metadata', async () => {
const client = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will work but is it better to create single mockedClient (which already exists as I remember) and use mockClient.someMethod.mockResolvedValueOnce everywhere without mocking everything inside tests

},
{
provide: RdiAnalytics,
useValue: {
Copy link
Collaborator

Choose a reason for hiding this comment

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

move to mocks

ArtemHoruzhenko
ArtemHoruzhenko previously approved these changes Jul 8, 2024
Copy link
Collaborator

@ArtemHoruzhenko ArtemHoruzhenko left a comment

Choose a reason for hiding this comment

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

Lgtm

Base automatically changed from feature/RI-5902-rdi-integration to main July 9, 2024 13:48
@vlad-dargel vlad-dargel dismissed ArtemHoruzhenko’s stale review July 9, 2024 13:48

The base branch was changed.

@ArtemHoruzhenko ArtemHoruzhenko merged commit fb3ef2a into main Jul 9, 2024
@ArtemHoruzhenko ArtemHoruzhenko deleted the be/feature/RI-5741-rdi-unit-tests branch July 9, 2024 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants