Skip to content

Add integration tests for RC manage version operations #914

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

Merged
merged 3 commits into from
Jun 26, 2020

Conversation

lahirumaramba
Copy link
Member

@lahirumaramba lahirumaramba commented Jun 23, 2020

  • Add integration tests for listVersions, getTemplateAtVersion, and rollback operations
  • Add integration tests for versions metadata in Remote Config templates
  • Remove undefined values in list versions options, and add unit tests
  • Check for empty versions lists in listVersions, and add unit tests

Note: Added release:stage to trigger integration tests. Merging to remote-config-vc master.

@lahirumaramba lahirumaramba added the release:stage Stage a release candidate label Jun 23, 2020
@lahirumaramba lahirumaramba marked this pull request as draft June 23, 2020 19:51
@lahirumaramba lahirumaramba marked this pull request as ready for review June 23, 2020 21:31
@lahirumaramba lahirumaramba requested a review from hiranya911 June 23, 2020 21:31
@lahirumaramba
Copy link
Member Author

Note: That failing integration test is from ML. Fix is in master

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 mostly good. Just a few tidbits to clean up.

},
];

const VALID_VERSION: admin.remoteConfig.Version = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering. Are these explicit type annotations really required?

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 removed the type annotations for VALID_VERSION. It looks like we need the type annotation for VALID_CONDITIONS unless I use a const INDIGO: admin.remoteConfig.TagColor = 'INDIGO' for tag color.

});
});

it('verfy that getTemplateAtVersion() returns the requested template version v2', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems redundant.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was to confirm that we are not getting the most recent template and having two versions confirms it. I see your point though :)
Will remove the expects here, but leave the templates for listVersions tests.

@lahirumaramba lahirumaramba force-pushed the lm-rc-vc-integration branch from 9ad71f8 to ca57fd4 Compare June 24, 2020 15:43
@lahirumaramba lahirumaramba requested a review from hiranya911 June 25, 2020 19:41
@lahirumaramba lahirumaramba changed the base branch from remote-config-vc to master June 25, 2020 23:40
@lahirumaramba lahirumaramba force-pushed the lm-rc-vc-integration branch from ca57fd4 to 28a22ff Compare June 25, 2020 23:49
@lahirumaramba lahirumaramba force-pushed the lm-rc-vc-integration branch from 28a22ff to be4d8dc Compare June 25, 2020 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:stage Stage a release candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants