damccorm commented on code in PR #17341: URL: https://github.com/apache/beam/pull/17341#discussion_r855155414
########## sdks/typescript/test/core_test.ts: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as beam from "../src/apache_beam"; +import * as assert from "assert"; +import { BytesCoder } from "../src/apache_beam/coders/standard_coders"; +import { Pipeline } from "../src/apache_beam/internal/pipeline"; +// TODO(pabloem): Fix installation. Review Comment: @pabloem what is this TODO? Can we either fix it or move it to a JIRA? ########## sdks/typescript/test/serialize_test.ts: ########## @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from "chai"; +import { + deserialize, + serialize, + BuiltinList, + generateDefaultBuiltins, +} from "serialize-closures"; + +describe("serialization tests", function () { + function roundtrip(value, builtins?: BuiltinList) { + return deserialize( + JSON.parse(JSON.stringify(serialize(value, builtins))), + builtins + ); + } + + function expectRoundtrip(value, builtins?: BuiltinList) { + expect(roundtrip(value, builtins)).to.deep.equal(value); + } + + function* myGenerator() { + yield 42; + yield 84; + } + + function simpleGenerator() { + expect(myGenerator().next()).to.equal(42); + } + + function roundtripGeneratorConstructor() { + expect(roundtrip(myGenerator)().next()).to.equal(42); + } + + function roundtripGeneratorInProgress() { Review Comment: @kerrydc it looks like these test aren't actually using the generator or these functions - did you mean to make this: ``` it("serializes and deserializes in progress generators", function() { const gen = myGenerator(); expect(gen.next()).to.equal(42); expect(roundtrip(gen).next()).to.equal(84); }); ``` Same question for the other generator related functions ########## sdks/typescript/test/combine_test.ts: ########## @@ -0,0 +1,276 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as beam from "../src/apache_beam"; +import { DirectRunner } from "../src/apache_beam/runners/direct_runner"; +import * as testing from "../src/apache_beam/testing/assert"; +import { KV } from "../src/apache_beam/values"; + +import { PortableRunner } from "../src/apache_beam/runners/portable_runner/runner"; +import * as combiners from "../src/apache_beam/transforms/combiners"; +import { + CombineFn, + GroupBy, + GroupGlobally, + CountPerElement, + CountGlobally, +} from "../src/apache_beam/transforms/group_and_combine"; + +describe("Apache Beam combiners", function () { + it("runs wordcount with a countPerKey transform and asserts the result", async function () { + // await new PortableRunner('localhost:3333').run( Review Comment: @pabloem Do we need this? ########## sdks/typescript/test/core_test.ts: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as beam from "../src/apache_beam"; +import * as assert from "assert"; +import { BytesCoder } from "../src/apache_beam/coders/standard_coders"; +import { Pipeline } from "../src/apache_beam/internal/pipeline"; +// TODO(pabloem): Fix installation. + +describe("core module", function () { + describe("runs a basic impulse expansion", function () { + it("runs a basic Impulse expansion", function () { + var p = new Pipeline(); + var res = new beam.Root(p).apply(new beam.Impulse()); + + assert.equal(res.type, "pcollection"); + assert.deepEqual(p.context.getPCollectionCoder(res), new BytesCoder()); + }); + it("runs a ParDo expansion", function () { + var p = new Pipeline(); + var res = new beam.Root(p) + .apply(new beam.Impulse()) + .map(function (v: any) { + return v * 2; + }); + }); + it("runs a GroupBy expansion", function () {}); Review Comment: @pabloem we should either cut this or fill it out ########## sdks/typescript/test/core_test.ts: ########## @@ -0,0 +1,44 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as beam from "../src/apache_beam"; +import * as assert from "assert"; +import { BytesCoder } from "../src/apache_beam/coders/standard_coders"; +import { Pipeline } from "../src/apache_beam/internal/pipeline"; +// TODO(pabloem): Fix installation. + +describe("core module", function () { + describe("runs a basic impulse expansion", function () { + it("runs a basic Impulse expansion", function () { + var p = new Pipeline(); + var res = new beam.Root(p).apply(new beam.Impulse()); + + assert.equal(res.type, "pcollection"); + assert.deepEqual(p.context.getPCollectionCoder(res), new BytesCoder()); + }); + it("runs a ParDo expansion", function () { + var p = new Pipeline(); + var res = new beam.Root(p) + .apply(new beam.Impulse()) + .map(function (v: any) { + return v * 2; + }); Review Comment: @pabloem Should we be doing some assertions here? ########## sdks/typescript/test/serialize_test.ts: ########## @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { expect } from "chai"; +import { + deserialize, + serialize, + BuiltinList, + generateDefaultBuiltins, +} from "serialize-closures"; + +describe("serialization tests", function () { + function roundtrip(value, builtins?: BuiltinList) { + return deserialize( + JSON.parse(JSON.stringify(serialize(value, builtins))), + builtins + ); + } + + function expectRoundtrip(value, builtins?: BuiltinList) { Review Comment: @kerrydc it doesn't look like this is used ########## sdks/typescript/test/combine_test.ts: ########## @@ -0,0 +1,276 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import * as beam from "../src/apache_beam"; +import { DirectRunner } from "../src/apache_beam/runners/direct_runner"; +import * as testing from "../src/apache_beam/testing/assert"; +import { KV } from "../src/apache_beam/values"; + +import { PortableRunner } from "../src/apache_beam/runners/portable_runner/runner"; +import * as combiners from "../src/apache_beam/transforms/combiners"; +import { + CombineFn, + GroupBy, + GroupGlobally, + CountPerElement, + CountGlobally, +} from "../src/apache_beam/transforms/group_and_combine"; + +describe("Apache Beam combiners", function () { + it("runs wordcount with a countPerKey transform and asserts the result", async function () { + // await new PortableRunner('localhost:3333').run( Review Comment: Same thing applies to the duplicate line in primitives_test and wordcount.ts -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@beam.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
