Skip to content

Add serverless: forbid for collection management tests #1216

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 4 commits into from
May 16, 2022

Conversation

jyemin
Copy link
Contributor

@jyemin jyemin commented May 16, 2022

Please complete the following before merging:

  • [n/a] Bump spec version and last modified date.
  • [n/a] Update changelog.
  • Make sure there are generated JSON files from the YAML test files.
  • Test changes in at least one language driver.
  • Test these changes against all server versions and topologies (including standalone, replica set, sharded clusters, and serverless).

Not sure why this wasn't caught in initial testing, but serverless tests are failing in Java driver.

@jyemin jyemin requested review from abr-egn and benjirewis May 16, 2022 13:44
@jyemin jyemin requested a review from a team as a code owner May 16, 2022 13:44
@jyemin jyemin requested review from kmahar and removed request for a team May 16, 2022 13:44
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Sounds good. I didn't catch these failures in Go because we only run a subset of unified tests against serverless; I assume Java is running all, @jyemin ?

I think you should also forbid serverless on timeseries-collection and modifyCollection-pre_and_post_images, too.

@jyemin
Copy link
Contributor Author

jyemin commented May 16, 2022

Java runs all unified tests on serverless, and relies on runOnRequirements to restrict. Everyone should do this IMO.

I think you should also forbid serverless on timeseries-collection and modifyCollection-pre_and_post_images, too

Those tests are not failing so I didn't change them. Is time series not supported on serverless? I don't know why modify... is succeeded, but it seems to be.

@benjirewis
Copy link
Contributor

Everyone should do this IMO.

Filed GODRIVER-2421. You are correct, and it says so in the spec for serverless testing here.

As long as the timeseries tests aren't failing I suppose it's fine to allow serverless on them. Go does not run the modifyCollection-pre_and_post_images test since we don't have a collMod helper; I assume Java is similar. If the createCollection tests are failing, then I bet the modifyCollection tests would fail for those that have the collMod helper, too.

@jyemin
Copy link
Contributor Author

jyemin commented May 16, 2022

Added serverless restriction to modify collection test

@jyemin jyemin requested a review from benjirewis May 16, 2022 15:05
Copy link
Contributor

@benjirewis benjirewis left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@jyemin jyemin merged commit d145882 into mongodb:master May 16, 2022
@jyemin jyemin deleted the coll-mgmt-serverles-forbid branch May 16, 2022 16:28
@jmikola
Copy link
Member

jmikola commented May 16, 2022

@jyemin: Was there a DRIVERS ticket for this change? I assume the new commit will be relevant for drivers to sync to.

If there will be a ticket created for this, I think it'd also be a good opportunity to knock out DRIVERS-2325 since the same test file will be synced.

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