Skip to content

Make Serializer tests work in browser, remove test duplication #2757

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.

Sign up for GitHub

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
Mar 18, 2020
Merged

Make Serializer tests work in browser, remove test duplication #2757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e8f6cd
Make Serializer tests work in browser, remove test duplication
schmidt-sebastian Mar 17, 2020
8aa9e15
Fix typos
schmidt-sebastian Mar 18, 2020
cfebaac
Empty line
schmidt-sebastian Mar 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/firestore/src/util/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export function isNullOrUndefined(value: unknown): boolean {
}

/** Returns whether the value represents -0. */
export function isNegativeZero(value: number) : boolean {
export function isNegativeZero(value: number): boolean {
// Detect if the value is -0.0. Based on polyfill from
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/is
return value === -0 && 1 / value === 1 / -0;
Expand Down
139 changes: 0 additions & 139 deletions packages/firestore/test/unit/model/field_value.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,145 +37,6 @@ describe('FieldValue', () => {
const date1 = new Date(2016, 4, 2, 1, 5);
const date2 = new Date(2016, 5, 20, 10, 20, 30);

it('can parse integers', () => {
const primitiveValues = [
Number.MIN_SAFE_INTEGER,
-1,
0,
1,
2,
Number.MAX_SAFE_INTEGER
];
const values = primitiveValues.map(v => wrap(v));

values.forEach(v => {
expect(v).to.be.an.instanceof(fieldValue.IntegerValue);
});

for (let i = 0; i < primitiveValues.length; i++) {
const primitiveValue = primitiveValues[i];
const value = values[i];
expect(value.value()).to.equal(primitiveValue);
}
});

it('can parse doubles', () => {
const primitiveValues = [
Number.MIN_SAFE_INTEGER - 1,
-1.1,
0.1,
Number.MAX_SAFE_INTEGER + 1,
NaN,
Infinity,
-Infinity
];
const values = primitiveValues.map(v =>
wrap(v)
) as fieldValue.NumberValue[];

values.forEach(v => {
expect(v).to.be.an.instanceof(fieldValue.DoubleValue);
});

for (let i = 0; i < primitiveValues.length; i++) {
const primitiveValue = primitiveValues[i];
const value = values[i];
if (isNaN(primitiveValue)) {
expect(isNaN(value.value())).to.equal(isNaN(primitiveValue));
} else {
expect(value.value()).to.equal(primitiveValue);
}
}
});

it('can parse null', () => {
const nullValue = wrap(null);

expect(nullValue).to.be.an.instanceof(fieldValue.NullValue);
expect(nullValue.value()).to.equal(null);

// check for identity for interning
expect(nullValue).to.equal(wrap(null));
});

it('can parse booleans', () => {
const trueValue = wrap(true);
const falseValue = wrap(false);

expect(trueValue).to.be.an.instanceof(fieldValue.BooleanValue);
expect(falseValue).to.be.an.instanceof(fieldValue.BooleanValue);

expect(trueValue.value()).to.equal(true);
expect(falseValue.value()).to.equal(false);

// check for identity for interning
expect(trueValue).to.equal(wrap(true));
expect(falseValue).to.equal(wrap(false));
});

it('can parse dates', () => {
const dateValue1 = wrap(date1);
const dateValue2 = wrap(date2);

expect(dateValue1).to.be.an.instanceof(fieldValue.TimestampValue);
expect(dateValue2).to.be.an.instanceof(fieldValue.TimestampValue);

expect(dateValue1.value()).to.deep.equal(Timestamp.fromDate(date1));
expect(dateValue2.value()).to.deep.equal(Timestamp.fromDate(date2));
});

it('can parse geo points', () => {
const latLong1 = new GeoPoint(1.23, 4.56);
const latLong2 = new GeoPoint(-20, 100);
const value1 = wrap(latLong1) as fieldValue.GeoPointValue;
const value2 = wrap(latLong2) as fieldValue.GeoPointValue;

expect(value1).to.be.an.instanceof(fieldValue.GeoPointValue);
expect(value2).to.be.an.instanceof(fieldValue.GeoPointValue);

expect(value1.value().latitude).to.equal(1.23);
expect(value1.value().longitude).to.equal(4.56);
expect(value2.value().latitude).to.equal(-20);
expect(value2.value().longitude).to.equal(100);
});

it('can parse bytes', () => {
const bytesValue = wrap(blob(0, 1, 2)) as fieldValue.BlobValue;

expect(bytesValue).to.be.an.instanceof(fieldValue.BlobValue);
expect(bytesValue.value().toUint8Array()).to.deep.equal(
new Uint8Array([0, 1, 2])
);
});

it('can parse simple objects', () => {
const objValue = wrap({ a: 'foo', b: 1, c: true, d: null });

expect(objValue).to.be.an.instanceof(fieldValue.ObjectValue);
expect(objValue.value()).to.deep.equal({
a: 'foo',
b: 1,
c: true,
d: null
});
});

it('can parse nested objects', () => {
const objValue = wrap({ foo: { bar: 1, baz: [1, 2, { a: 'b' }] } });

expect(objValue).to.be.an.instanceof(fieldValue.ObjectValue);
expect(objValue.value()).to.deep.equal({
foo: { bar: 1, baz: [1, 2, { a: 'b' }] }
});
});

it('can parse empty objects', () => {
const objValue = wrap({ foo: {} });

expect(objValue).to.be.an.instanceof(fieldValue.ObjectValue);
expect(objValue.value()).to.deep.equal({ foo: {} });
});

it('can extract fields', () => {
const objValue = wrapObject({ foo: { a: 1, b: true, c: 'string' } });

Expand Down
122 changes: 41 additions & 81 deletions .../test/unit/remote/node/serializer.test.ts → ...store/test/unit/remote/serializer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,12 @@
*/

import { expect } from 'chai';
import * as Long from 'long';
import * as ProtobufJS from 'protobufjs';

import { Blob } from '../../../../src/api/blob';
import { PublicFieldValue as FieldValue } from '../../../../src/api/field_value';
import { GeoPoint } from '../../../../src/api/geo_point';
import { Timestamp } from '../../../../src/api/timestamp';
import { DatabaseId } from '../../../../src/core/database_info';

import { Blob } from '../../../src/api/blob';
import { PublicFieldValue as FieldValue } from '../../../src/api/field_value';
import { GeoPoint } from '../../../src/api/geo_point';
import { Timestamp } from '../../../src/api/timestamp';
import { DatabaseId } from '../../../src/core/database_info';
import {
ArrayContainsAnyFilter,
ArrayContainsFilter,
Expand All @@ -34,35 +32,30 @@ import {
Operator,
OrderBy,
Query
} from '../../../../src/core/query';
import { SnapshotVersion } from '../../../../src/core/snapshot_version';
import { Target } from '../../../../src/core/target';
import { TargetData, TargetPurpose } from '../../../../src/local/target_data';
import * as fieldValue from '../../../../src/model/field_value';
} from '../../../src/core/query';
import { SnapshotVersion } from '../../../src/core/snapshot_version';
import { Target } from '../../../src/core/target';
import { TargetData, TargetPurpose } from '../../../src/local/target_data';
import * as fieldValue from '../../../src/model/field_value';
import {
DeleteMutation,
FieldMask,
Mutation,
Precondition,
SetMutation,
VerifyMutation
} from '../../../../src/model/mutation';
import { DOCUMENT_KEY_NAME, FieldPath } from '../../../../src/model/path';
import {
loadRawProtos,
protoLoaderOptions
} from '../../../../src/platform_node/load_protos';
import * as api from '../../../../src/protos/firestore_proto_api';
import { JsonProtoSerializer } from '../../../../src/remote/serializer';
} from '../../../src/model/mutation';
import { DOCUMENT_KEY_NAME, FieldPath } from '../../../src/model/path';
import * as api from '../../../src/protos/firestore_proto_api';
import { JsonProtoSerializer } from '../../../src/remote/serializer';
import {
DocumentWatchChange,
WatchTargetChange,
WatchTargetChangeState
} from '../../../../src/remote/watch_change';
import { Code, FirestoreError } from '../../../../src/util/error';
import { Indexable } from '../../../../src/util/misc';
import * as obj from '../../../../src/util/obj';
import { addEqualityMatcher } from '../../../util/equality_matcher';
} from '../../../src/remote/watch_change';
import { Code, FirestoreError } from '../../../src/util/error';
import * as obj from '../../../src/util/obj';
import { addEqualityMatcher } from '../../util/equality_matcher';
import {
bound,
byteStringFromString,
Expand All @@ -82,24 +75,26 @@ import {
version,
wrap,
wrapObject
} from '../../../util/helpers';
import { ByteString } from '../../../../src/util/byte_string';
} from '../../util/helpers';
import { ByteString } from '../../../src/util/byte_string';
import { isNode } from '../../util/test_platform';

let verifyProtobufJsRoundTrip: (jsonValue: api.Value) => void = () => {};

if (isNode()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you could just inline the definition of verifyProtobufJsRoundTrip here. It doesn't seem like it would be useful in any other context because it hard codes the use of ValueMessage.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put in a separate file so we could import ProtobufJs and its types using "import" instead of "require". This gives us type safety. If I included it here, I would have to rely on the d.t.s files for these dependencies to use the types safely, without importing the actual sources (an import of ProtobufJs here would break the build for the browser tests)..

// Note: We cannot use dynamic imports since our Node build uses CJS as its
// module syntax.
// eslint-disable-next-line @typescript-eslint/no-require-imports
verifyProtobufJsRoundTrip = require('../../util/node_helpers')
.verifyProtobufJsRoundTrip;
}

describe('Serializer', () => {
const partition = new DatabaseId('p', 'd');
const s = new JsonProtoSerializer(partition, { useProto3Json: false });
const proto3JsonSerializer = new JsonProtoSerializer(partition, {
useProto3Json: true
});
const protos = loadRawProtos();

// tslint:disable:variable-name
const ValueMessage = protos.lookupType('google.firestore.v1.Value');
const LatLngMessage = protos.lookupType('google.type.LatLng');
const TimestampMessage = protos.lookupType('google.protobuf.Timestamp');
const ArrayValueMessage = protos.lookupType('google.firestore.v1.ArrayValue');
const MapValueMessage = protos.lookupType('google.firestore.v1.MapValue');
// tslint:enable:variable-name

/**
* Wraps the given target in TargetData. This is useful because the APIs we're
Expand All @@ -126,24 +121,13 @@ describe('Serializer', () => {
valueType: string;
/** The expected JSON value for the field (e.g. 'NULL_VALUE') */
jsonValue: unknown;
/**
* The expected protobufJs value for the field (e.g. `0`). This is
* largely inconsequential (we only rely on the JSON representation), but
* it can be useful for debugging issues. If omitted, it's assumed to be
* the same as jsonValue.
*/
protobufJsValue?: unknown;
/**
* If true, uses the proto3Json serializer (and skips the round-trip
* through protobufJs).
*/
useProto3Json?: boolean;
}): void {
const { value, valueType, jsonValue } = opts;
const protobufJsValue =
opts.protobufJsValue !== undefined
? opts.protobufJsValue
: opts.jsonValue;
const serializer = opts.useProto3Json ? proto3JsonSerializer : s;

// Convert FieldValue to JSON and verify.
Expand All @@ -152,20 +136,7 @@ describe('Serializer', () => {

// If we're using protobufJs JSON (not Proto3Json), then round-trip through protobufjs.
if (!opts.useProto3Json) {
// Convert JSON to protobufjs and verify value.
const actualProtobufjsProto: ProtobufJS.Message = ValueMessage.fromObject(
actualJsonProto
);
expect(
((actualProtobufjsProto as unknown) as Indexable)[valueType]
).to.deep.equal(protobufJsValue);

// Convert protobufjs back to JSON.
const returnJsonProto = ValueMessage.toObject(
actualProtobufjsProto,
protoLoaderOptions
);
expect(returnJsonProto).to.deep.equal(actualJsonProto);
verifyProtobufJsRoundTrip(actualJsonProto);
}

// Convert JSON back to FieldValue.
Expand All @@ -177,8 +148,7 @@ describe('Serializer', () => {
verifyFieldValueRoundTrip({
value: fieldValue.NullValue.INSTANCE,
valueType: 'nullValue',
jsonValue: 'NULL_VALUE',
protobufJsValue: 0
jsonValue: 'NULL_VALUE'
});
});

Expand Down Expand Up @@ -207,11 +177,7 @@ describe('Serializer', () => {
verifyFieldValueRoundTrip({
value: new fieldValue.IntegerValue(example),
valueType: 'integerValue',
jsonValue: '' + example,
protobufJsValue: Long.fromString(
example.toString(),
/*unsigned=*/ false
)
jsonValue: '' + example
});
}
});
Expand Down Expand Up @@ -271,8 +237,7 @@ describe('Serializer', () => {
verifyFieldValueRoundTrip({
value: new fieldValue.TimestampValue(Timestamp.fromDate(examples[i])),
valueType: 'timestampValue',
jsonValue: expectedJson[i],
protobufJsValue: TimestampMessage.fromObject(expectedJson[i])
jsonValue: expectedJson[i]
});
}
});
Expand Down Expand Up @@ -345,8 +310,7 @@ describe('Serializer', () => {
verifyFieldValueRoundTrip({
value: new fieldValue.GeoPointValue(example),
valueType: 'geoPointValue',
jsonValue: expected,
protobufJsValue: LatLngMessage.fromObject(expected)
jsonValue: expected
});
});

Expand Down Expand Up @@ -380,26 +344,23 @@ describe('Serializer', () => {
verifyFieldValueRoundTrip({
value,
valueType: 'arrayValue',
jsonValue,
protobufJsValue: ArrayValueMessage.fromObject(jsonValue)
jsonValue
});
});

it('converts empty ArrayValue', () => {
verifyFieldValueRoundTrip({
value: wrap([]),
valueType: 'arrayValue',
jsonValue: { values: [] },
protobufJsValue: ArrayValueMessage.fromObject({})
jsonValue: { values: [] }
});
});

it('converts ObjectValue.EMPTY', () => {
verifyFieldValueRoundTrip({
value: wrap({}),
valueType: 'mapValue',
jsonValue: { fields: {} },
protobufJsValue: MapValueMessage.fromObject({})
jsonValue: { fields: {} }
});
});

Expand Down Expand Up @@ -467,8 +428,7 @@ describe('Serializer', () => {
verifyFieldValueRoundTrip({
value: objValue,
valueType: 'mapValue',
jsonValue: expectedJson.mapValue,
protobufJsValue: MapValueMessage.fromObject(expectedJson.mapValue!)
jsonValue: expectedJson.mapValue
});
});

Expand Down
Loading