Skip to content
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

feat: Support Logical Termination on RunQueryResponse #1741

Merged
merged 3 commits into from
Jun 30, 2022
Merged

Conversation

cherylEnkidu
Copy link
Contributor

Logical Termination: RunQueryResponse will emit "end" event when the "data" event proto which has "done" field set to true.

@cherylEnkidu cherylEnkidu requested review from a team as code owners June 23, 2022 21:52
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. api: firestore Issues related to the googleapis/nodejs-firestore API. labels Jun 23, 2022
@@ -2024,6 +2025,9 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
}
})
.on('end', () => {
if (hasResolve) return;
hasResolve = true;
Copy link
Contributor

Choose a reason for hiding this comment

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

hasResolved

Also, did you check whether you can just invoke resolve() again?

})
.on('end', () => {
expect(received).to.equal(2);
expect(++counter).to.equal(1);
Copy link
Contributor

@schmidt-sebastian schmidt-sebastian Jun 23, 2022

Choose a reason for hiding this comment

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

You should test that you only receive a single 'end' callback. One think you can do is change this to:

let endCounter = 0;
query
        .stream()
        .on('data', doc => {
          expect(doc).to.be.an.instanceOf(DocumentSnapshot);
          ++receivedCounter;
        })
        .on('end', () => {
          expect(received).to.equal(2);
         ++endCounter;
          setImmediate(() => {
             expect(endCounter).to.equal(1);
             callback();
          });
        });
    });
  });

This way, you ensure that once all test processing is done, there is an additional check that verifies that 'end' is only called once. Your test right now does the same thing, but once you call callback test is considered done and it won't fail even if any expectations fail after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/nodejs-firestore API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants