Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(server): incorrect prisma overriding #5857

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

L-Sun
Copy link
Contributor

@L-Sun L-Sun commented Feb 21, 2024

In some backend server tests, overrideProvider(PrismaClient).useClass(FakePrisma) dose not have effect.

test.beforeEach(async t => {
const { app } = await createTestingApp({
imports: [AppModule],
tapModule(builder) {
builder
.overrideProvider(PrismaClient)
.useClass(FakePrisma)
.overrideProvider(FeatureManagementService)
.useValue({ canEarlyAccess: () => true });
},
});

test.beforeEach(async t => {
const { module, app } = await createTestingApp({
imports: [AppModule],
tapModule: module => {
module
.overrideProvider(PrismaClient)
.useValue(FakePrisma)
.overrideProvider(FeatureManagementService)
.useValue({
hasWorkspaceFeature() {
return false;
},
});
},
});

the Injectable may only effect the PrismaService not PrismaClient

@Injectable()
export class PrismaService
extends PrismaClient
implements OnModuleInit, OnModuleDestroy
{
static INSTANCE: PrismaService | null = null;

In mailer.spec.ts, however, it seem some methods are missing in FakePrisma, which led to the test failing after fixing the override problem. So I keep this code unchanged.

The test of mailer.spec.ts seems to only test the success of sending emails. I think there is no need to mock Prisma behavior. Just providing some fake data to the send function is good.

const resp = await mail.sendInviteEmail(
'production@toeverything.info',
inviteId,
{
workspace: {
id: inviteInfo.workspace.id,
name: inviteInfo.workspace.name,
avatar: '',
},
user: {
avatar: inviteInfo.user?.avatarUrl || '',
name: inviteInfo.user?.name || '',
},
}
);
t.is(resp.accepted.length, 1, 'failed to send invite email');
}
t.pass();

@CLAassistant
Copy link

CLAassistant commented Feb 21, 2024

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added app:server test Related to test cases labels Feb 21, 2024
@EYHN EYHN requested a review from forehalo February 22, 2024 02:46
@forehalo
Copy link
Member

forehalo commented Feb 22, 2024

Hi @L-Sun, thanks for your contribution. It's my mistake.

I previously made PrismaService as an alias of PrismaClient from @prisma/client for easier importing(we don't have path alias so sometime we might need a lot of parent directory seeking like '../../../prisma' for importing PrismaService)

The problem is I didn't replace all PrismaService with PrismaClient at once, which seems to be a bad decision now. I will continue doing the replacing and once I finish it, I will also forbid the using of PrismaService.

Copy link

nx-cloud bot commented Feb 22, 2024

Copy link

codecov bot commented Feb 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (201f680) 64.51% compared to head (98f7132) 64.72%.
Report is 7 commits behind head on canary.

Additional details and impacted files
@@            Coverage Diff             @@
##           canary    #5857      +/-   ##
==========================================
+ Coverage   64.51%   64.72%   +0.21%     
==========================================
  Files         360      358       -2     
  Lines       19956    19907      -49     
  Branches     1701     1697       -4     
==========================================
+ Hits        12874    12885      +11     
+ Misses       6863     6803      -60     
  Partials      219      219              
Flag Coverage Δ
server-test 71.57% <100.00%> (+0.21%) ⬆️
unittest 45.39% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@forehalo forehalo changed the title fix(test): incorrect prisma overriding test(server): incorrect prisma overriding Feb 22, 2024
Copy link

graphite-app bot commented Feb 22, 2024

Merge activity

@forehalo forehalo merged commit 46cc081 into toeverything:canary Feb 22, 2024
34 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
app:server test Related to test cases
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants