Fix Integration tests against 'minified' sources #2445
Conversation
| @@ -159,7 +158,7 @@ apiDescribe('Validation:', (persistence: boolean) => { | |||
|
|
|||
| validationIt(persistence, 'garbage collection can be disabled', db => { | |||
| // Verify that this doesn't throw. | |||
| db.settings({ cacheSizeBytes: CACHE_SIZE_UNLIMITED }); | |||
| db.settings({ cacheSizeBytes: /* CACHE_SIZE_UNLIMITED */ -1 }); | |||
There was a problem hiding this comment.
The constant is not exported internally (https://github.com/firebase/firebase-js-sdk/blob/ae319059e4a083a613a2999c1e61d44c9053a0d7/packages/firestore/index.ts)
There was a problem hiding this comment.
Does that mean our types / docs are a lie?
firebase-js-sdk/packages/firebase/index.d.ts
Line 6986 in 6480d7f
If so, can you open a bug?
There was a problem hiding this comment.
I was able to make it work by importing it from the namespace: firestore.CACHE_SIZE_UNLIMITED
| () => fail('Expected cache miss'), | ||
| err => expect(err.code).to.be.equal(Code.UNAVAILABLE) | ||
| () => { | ||
| throw new Error('Expected cache miss'); |
There was a problem hiding this comment.
I think you can use expect.fail(...)
There was a problem hiding this comment.
Fixed here and in the rest of the file.
| } | ||
| ); | ||
| return deferred.promise; | ||
| return withTestCollection(persistence, {}, queryForRejection => { |
There was a problem hiding this comment.
Eh... I think the old (disabled) code was barely discernible and the new disabled code crosses the line into indiscernible territory.
I think it might be better to change the original line to:
const queryForRejection = null as Query;
And then the TODO makes it barely discernible again that queryForRejection is supposed to be a query that will be rejected.
There was a problem hiding this comment.
Done. I wonder though if we could just use a query that requires a non-existing index.
There was a problem hiding this comment.
Oh! That's a good thought.
| @@ -159,7 +158,7 @@ apiDescribe('Validation:', (persistence: boolean) => { | |||
|
|
|||
| validationIt(persistence, 'garbage collection can be disabled', db => { | |||
| // Verify that this doesn't throw. | |||
| db.settings({ cacheSizeBytes: CACHE_SIZE_UNLIMITED }); | |||
| db.settings({ cacheSizeBytes: /* CACHE_SIZE_UNLIMITED */ -1 }); | |||
There was a problem hiding this comment.
Does that mean our types / docs are a lie?
firebase-js-sdk/packages/firebase/index.d.ts
Line 6986 in 6480d7f
If so, can you open a bug?
This PR fixes
npm run test:manualin 'integration/firestore'.It moves all tests that require Internal API to the respective "api_internal" tests. It also does the same for the helpers. For the timestampsInSnapshot tests, it unfortunately re-adds the logging to the console output (running a test that verifies that we don't minify our settings is probably a good enough justification).
Two follow up PRs for this: