Skip to content

Import firestore conformance tests from googleapis/conformance-tests@32fd66cf7dffe8431643f08b5944108ac10c8bc2 - #5541

Merged
BenWhitehead merged 1 commit into
googleapis:masterfrom
BenWhitehead:firestore/conformance-tests-migration/step2
Jul 10, 2019
Merged

Import firestore conformance tests from googleapis/conformance-tests@32fd66cf7dffe8431643f08b5944108ac10c8bc2#5541
BenWhitehead merged 1 commit into
googleapis:masterfrom
BenWhitehead:firestore/conformance-tests-migration/step2

Conversation

@BenWhitehead

@BenWhiteheadBenWhitehead commented Jun 21, 2019

Copy link
Copy Markdown
Contributor
  • Add new script generate-conformance-tests.sh that can reliably update
    the various files that make up the conformance tests.
    * Currently the script only updates firestore tests.
    * Updating of the git-submodule linking to conformance-tests is not
    done by the script, it is expected that the developer will do the
    bumping.
  • import proto definition
  • generate updated TestDefinition from definition
  • Add individual test files
  • Add maven profile gen-conformance-protos that can be used to generate protos in com.google.cloud:google-cloud-conformance-tests

Depends on #5540

@BenWhitehead
BenWhitehead requested a review from a teamJune 21, 2019 22:26
@googlebotgooglebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 21, 2019
@BenWhitehead

BenWhitehead commented Jun 21, 2019

Copy link
Copy Markdown
ContributorAuthor

@kolea2 Please review

@BenWhiteheadBenWhitehead mentioned this pull request Jun 21, 2019
Merged
chingor13
chingor13 reviewed Jun 21, 2019
View reviewed changes

@chingor13chingor13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are all the languages using submodules for reading the tests?

@BenWhitehead

BenWhitehead commented Jun 24, 2019

Copy link
Copy Markdown
ContributorAuthor

Are all the languages using submodules for reading the tests?

They are starting to move in that direction. dotnet is already using sub-module, and ruby is going to move when updating the firestore tests.

I will note, the submodule here is purely to track the commit the resources are from, the sub-module does not need to be cloned for the tests to run. The script generate-conformance-tests.sh copies and generates everything into the correct location so the tests can run.

@codecov

codecovBot commented Jun 27, 2019•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #5541 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #5541 +/- ##
============================================
- Coverage 46.71% 46.71% -0.01% + Complexity 24630 24624 -6 
============================================
Files 2351 2351 Lines 256127 256121 -6 Branches 29325 29323 -2 ============================================
- Hits 119650 119645 -5 + Misses 127558 127557 -1 
Partials 8919 8919
Impacted FilesCoverage ΔComplexity Δ
...able/gaxx/reframing/ReframingResponseObserver.java88.99% <0%> (-1.84%)29% <0%> (-1%)
...om/google/cloud/bigtable/data/v2/models/Query.java66.32% <0%> (-0.35%)22% <0%> (-3%)
...ogle/cloud/talent/v4beta1/TenantServiceClient.java59.25% <0%> (ø)22% <0%> (ø)⬇️
...oogle/cloud/talent/v4beta1/EventServiceClient.java42.42% <0%> (ø)7% <0%> (ø)⬇️
.../google/cloud/talent/v4beta1/JobServiceClient.java45.69% <0%> (ø)34% <0%> (ø)⬇️
...cloud/talent/v4beta1/ApplicationServiceClient.java61.17% <0%> (ø)23% <0%> (ø)⬇️
...gle/cloud/talent/v4beta1/CompanyServiceClient.java59.25% <0%> (ø)22% <0%> (ø)⬇️
...gle/cloud/talent/v4beta1/ProfileServiceClient.java47.52% <0%> (ø)22% <0%> (ø)⬇️
...src/main/java/com/google/cloud/ServiceOptions.java40.61% <0%> (+0.35%)27% <0%> (-2%)⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5d709c...c5dce8f. Read the comment docs.

@BenWhiteheadBenWhitehead changed the title Import firestore conformance tests from googleapis/conformance-tests@5a984706af594b8ec2ef549040139c9704554bf4Import firestore conformance tests from googleapis/conformance-tests@32fd66cf7dffe8431643f08b5944108ac10c8bc2Jun 27, 2019
@BenWhitehead

BenWhitehead commented Jun 27, 2019

Copy link
Copy Markdown
ContributorAuthor

The builds are currently failing because I need to rebase them. Since this PR is dependent on another PR I'll wait to rebase until that PR is merged. The changes in the PR are unrelated to the build failures and can still be reviewed.

@BenWhitehead

BenWhitehead commented Jun 28, 2019

Copy link
Copy Markdown
ContributorAuthor

Rebased after merge of #5540 and ready for review.

@yoshi-automationyoshi-automation added the 🚨 critical P0 critical issue. Requires immediate fix label Jun 28, 2019
kolea2
kolea2 reviewed Jul 1, 2019
View reviewed changes
Comment thread.gitmodules
@@ -0,0 +1,3 @@
[submodule "google-cloud-testing/google-cloud-conformance-tests/conformance-tests"]
path = google-cloud-testing/google-cloud-conformance-tests/conformance-tests
url = git@github.com:googleapis/conformance-tests.git

@kolea2kolea2Jul 1, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just for my understanding, what does this do?

@BenWhiteheadBenWhiteheadJul 1, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This file is the definition for the git-submodule which is being used to track which revision the local conformance test resources came from. You can see an example of what it looks like and how it exists in the file tree here.

@kolea2

kolea2 commented Jul 1, 2019

Copy link
Copy Markdown
Contributor

(can't comment on the file as it is too big) Is TestDefinition.java generated or hand written?

@BenWhitehead

BenWhitehead commented Jul 1, 2019

Copy link
Copy Markdown
ContributorAuthor

(can't comment on the file as it is too big) Is TestDefinition.java generated or hand written?

TestDefinition.java is generated from tests.proto.

@kolea2

kolea2 commented Jul 2, 2019

Copy link
Copy Markdown
Contributor

LGTM, @chingor13 do you have any final comments?

@chingor13

chingor13 commented Jul 2, 2019

Copy link
Copy Markdown
Contributor

LGTM, but can you add a bit more info on the README for the conformance tests on the relationship between the test protos and the json config files?

@yoshi-automationyoshi-automation removed the 🚨 critical P0 critical issue. Requires immediate fix label Jul 3, 2019
@BenWhitehead
Import firestore conformance tests fromgoogleapis/conformance-tests@3…
c5dce8f
…2fd66c
* Add new script generate-conformance-tests.sh that can reliably update
the various files that make up the conformance tests.
* Currently the script only updates firestore tests.
* Updating of the git-submodule linking to conformance-tests is not
done by the script, it is expected that the developer will do the
bumping.
* import proto definition
* generate updated TestDefinition from definition
* Add individual test files
* Add maven profile `gen-conformance-protos` that can be used to generate protos in `com.google.cloud:google-cloud-conformance-tests`
@BenWhitehead

BenWhitehead commented Jul 8, 2019

Copy link
Copy Markdown
ContributorAuthor

@chingor13 Added some details to the README to explain the files and their relationships. I've also rebased the PR on master.

@BenWhitehead
BenWhitehead merged commit 722e79c into googleapis:masterJul 10, 2019
@BenWhitehead
BenWhitehead deleted the firestore/conformance-tests-migration/step2 branch July 10, 2019 15:26
@kolea2kolea2 mentioned this pull request Jul 10, 2019
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chingor13chingor13chingor13 approved these changes
+1 more reviewer
@kolea2kolea2kolea2 left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yesThis human has signed the Contributor License Agreement.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@BenWhitehead@kolea2@chingor13@googlebot@yoshi-automation