test: Unit tests for Function App Service #156
Conversation
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Added some feedback. Let me know if you have any questions.
| AzureLoginService.servicePrincipalLogin = servicePrincipalLogin; | ||
|
|
||
| const sls = MockFactory.createTestServerless(); | ||
| delete sls.variables['azureCredentials'] |
There was a problem hiding this comment.
Can we move these into a beforeEach or similar to keep it DRY?
There was a problem hiding this comment.
The only issue with that is the deletion of the credentials is specific to these tests only. Other tests depend on the credentials still being there
| } | ||
| } | ||
|
|
||
| protected log(message: string) { |
|
|
||
| private static createTestService(): Service { | ||
| public static createTestFunctionApp() { | ||
| return { |
There was a problem hiding this comment.
We should add return typings here.
| webAppDelete = jest.fn(); | ||
| sendFile = jest.fn(); | ||
|
|
||
| WebSiteManagementClient.prototype.webApps = { |
There was a problem hiding this comment.
These methods can be mocked by mocking the sub object directly. ex) WebApp.prototype.get = jest.fn(...)
| beforeAll(() => { | ||
|
|
||
| // TODO: How to spy on defaul exported function? | ||
| const axiosMock = new MockAdapter(axios); |
There was a problem hiding this comment.
Do you find this mock adapter useful? I have mixed feelings.
There was a problem hiding this comment.
Yes and no... If I could make assertions on it, it would be amazing. I had to do something fairly hacky to validate the syncTriggers. I wasn't able to spy on axios directly. Maybe we can chat more about how to do that
src/test/mockFactory.ts
Outdated
| } as any as Service; | ||
| } | ||
|
|
||
| public static createTestFunctions(functionCount = 3) { |
There was a problem hiding this comment.
We should add typings to these mock factory functions.
There was a problem hiding this comment.
Part of the reason for not having typings is my own not knowing what types they are 😆
But agreed. I'll do some more digging here
tbarlow12
force-pushed
the
tabarlow/function-app-service-tests
branch
from
74304a8 to
ede2161
Compare
wbreza
left a comment
wbreza
left a comment
There was a problem hiding this comment.
Saw a couple things that need attention before merging.
src/models/index.ts
Outdated
| @@ -0,0 +1,4 @@ | |||
| export * from "./azureProvider"; | |||
There was a problem hiding this comment.
To make model imports easy. I can remove these
src/models/serverless.ts
Outdated
| @@ -0,0 +1,8 @@ | |||
| export interface ServerlessYml { | |||
There was a problem hiding this comment.
I would remove Yml and maybe rename to ServerlessAzureConfig or similar.
| it("sets up provider configuration", async () => { | ||
| const slsFunctionConfig = MockFactory.createTestSlsFunctionConfig(); | ||
| const sls = MockFactory.createTestServerless(); | ||
| Object.assign(sls.service, { |
There was a problem hiding this comment.
How do the functions config set if you are removing this?
There was a problem hiding this comment.
Looks like with the updates to mock factory you need to now pass into createTestServerless ??
There was a problem hiding this comment.
For this instance in particular, since it's just using the createTestSlsFunctionConfig out of the box, that will already be in the sls object. If there is a different function config to pass, that can be achieved by doing:
createTestServerless({
service: createTestService(functionConfig)
})
src/services/functionAppService.ts
Outdated
| } | ||
|
|
||
| public async getMasterKey(functionApp) { | ||
| public async getMasterKey(functionApp?) { |
|
|
||
| beforeEach(() => { | ||
| const slsConfig: any = { | ||
| ...MockFactory.createTestService(MockFactory.createTestSlsFunctionConfig()), |
There was a problem hiding this comment.
@wbreza this is another option for using a custom config
PIC123
left a comment
PIC123
left a comment
There was a problem hiding this comment.
Looks good! Much cleaner now too!
Resolves [AB#18737]