Skip to content

Conversation

lahirumaramba
Copy link
Member

  • Add integrations test for the RC API
  • Add a function to validate templates passed into validateTemplate() and publishTemplate() functions
  • Add unit tests for the new validations

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Mostly looks good. Few suggestions on eliminating redundant tests and keeping things simple.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Looks good. Let's move the argument validation tests into unit tests so they get tested more often.

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

Couple of points to improve on.

});

INVALID_CONDITIONS.forEach((invalidConditions) => {
console.log(inputTemplate);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks!

});

// validate input template
testInputTemplate((t: RemoteConfigTemplate) => { remoteConfig.validateTemplate(t); });
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't you just do testInputTemplate(remoteConfig.validateTemplate);

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried that but it gives me the following type error. Not quite sure why.

    testInvalidInputTemplates(remoteConfig.validateTemplate);
                                           ^
TypeError: Cannot read property 'validateTemplate' of undefined

});
});

function testInputTemplate(rcOperation: Function): void {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use a more descriptive name. Perhaps testInvalidInputTemplates()?

Copy link
Contributor

@hiranya911 hiranya911 left a comment

Choose a reason for hiding this comment

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

LGTM

@lahirumaramba lahirumaramba merged commit e207018 into remote-config Apr 2, 2020
@lahirumaramba lahirumaramba deleted the lm-remote-config-itests branch April 2, 2020 15:13
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.

2 participants