You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by GitBox <gi...@apache.org> on 2021/10/17 00:46:42 UTC
[GitHub] [daffodil-vscode] stevedlawrence commented on a change in pull request #33: Unit Test CI
stevedlawrence commented on a change in pull request #33:
URL: https://github.com/apache/daffodil-vscode/pull/33#discussion_r729834128
##########
File path: .github/workflows/tests.yml
##########
@@ -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.
+
+---
+name: Unit Tests
+on:
+ push:
+ branches-ignore: [ 'dependabot/**' ]
+ pull_request:
+ types: [opened, synchronize, reopened]
+
+jobs:
+ build:
+ name: Unit Tests
+ strategy:
+ matrix:
+ os: [macos-latest, ubuntu-latest, windows-latest]
+ runs-on: ${{ matrix.os }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v2
+ - name: Install Node.js
+ uses: actions/setup-node@v1
+ with:
+ node-version: 10.x
+ - run: npm install
+ - run: xvfb-run -a npm test
+ if: runner.os == 'Linux'
Review comment:
For my own curiosity, why does linux need this xvfb-run thing? Does `npm test` not work on linux?
##########
File path: .github/workflows/tests.yml
##########
@@ -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.
+
+---
+name: Unit Tests
+on:
+ push:
+ branches-ignore: [ 'dependabot/**' ]
+ pull_request:
+ types: [opened, synchronize, reopened]
+
+jobs:
+ build:
+ name: Unit Tests
+ strategy:
+ matrix:
+ os: [macos-latest, ubuntu-latest, windows-latest]
+ runs-on: ${{ matrix.os }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v2
+ - name: Install Node.js
+ uses: actions/setup-node@v1
+ with:
+ node-version: 10.x
+ - run: npm install
+ - run: xvfb-run -a npm test
+ if: runner.os == 'Linux'
+ - run: sbt compile headerCheck
Review comment:
This is fine for now, but this probably wants to be replaced when Rat check is added.
##########
File path: .github/workflows/tests.yml
##########
@@ -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.
+
+---
+name: Unit Tests
+on:
+ push:
+ branches-ignore: [ 'dependabot/**' ]
+ pull_request:
+ types: [opened, synchronize, reopened]
+
+jobs:
+ build:
+ name: Unit Tests
+ strategy:
+ matrix:
+ os: [macos-latest, ubuntu-latest, windows-latest]
+ runs-on: ${{ matrix.os }}
+ steps:
+ - name: Checkout
+ uses: actions/checkout@v2
Review comment:
Please use a full version. This ensures we are locked at a specific version and updates won't break things. Dependabot will sent us pull requests to update when new versions are out. Same with the Install Node.js step.
##########
File path: src/tests/daffodil.test.ts
##########
@@ -0,0 +1,122 @@
+/*
+ * 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 assert from 'assert'
+import * as daffodil from '../daffodil'
+
+suite('Daffodfil', () => {
+ // suite to test all functions work properly
+ suite('interfaces', () => {
+ test('DaffodilData functions properly', () => {
+ let daffodilData: daffodil.DaffodilData = {
+ bytePos1b: 100,
+ }
+
+ assert.strictEqual(100, daffodilData.bytePos1b)
+ })
+
+ test('InfosetEvent functions properly', () => {
+ let infosetEvent: daffodil.InfosetEvent = {
+ content: 'This is content',
+ mimeType: 'xml',
+ }
+
+ assert.strictEqual('This is content', infosetEvent.content)
+ assert.strictEqual('xml', infosetEvent.mimeType)
+ })
+
+ test('InfosetOutput functions properly', () => {
+ let infosetOutput: daffodil.InfosetOutput = {
+ type: 'console',
+ }
+
+ assert.strictEqual('console', infosetOutput.type)
+ })
+
+ test('BuildInfo functions properly', () => {
+ let allVersions = '1.0.0'
+ let buildInfo: daffodil.BuildInfo = {
+ version: allVersions,
+ daffodilVersion: allVersions,
+ scalaVersion: allVersions,
+ }
+
+ assert.strictEqual(allVersions, buildInfo.version)
+ assert.strictEqual(allVersions, buildInfo.daffodilVersion)
+ assert.strictEqual(allVersions, buildInfo.scalaVersion)
+ })
Review comment:
It's not clear to me what the tests in this file are testing. Seems like they just create instances of interfaces and validate that they were created correctly and that you can read the values you just set? What kind of changes would cause this to fail?
##########
File path: src/tests/data/DFDLGeneralFormat.dfdl.xsd
##########
@@ -0,0 +1,117 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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.
+-->
+
+<xs:schema xmlns:dfdl="http://www.ogf.org/dfdl/dfdl-1.0/"
Review comment:
I wouldn't recommend using this file as a test. This isn't really a schema that can be used to parse anything. Suggest something really simple, like a schema with a single xs:string element that's fixed length or something. Doesn't have to be complex.
##########
File path: src/tests/data/DFDLGeneralFormat.dfdl.xsd
##########
@@ -0,0 +1,117 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ 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
Review comment:
Missing empty lines around the URL in this copyright header.
##########
File path: src/tests/adapter.test.ts
##########
@@ -0,0 +1,99 @@
+/*
+ * 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 assert from 'assert'
+import * as path from 'path'
+import { DebugClient } from 'vscode-debugadapter-testsupport'
+
+suite('Daffodil Debug Adapter', () => {
+ const DEBUG_ADAPTER = './out/adapter/debugAdapter.js'
+ const PROJECT_ROOT = path.join(__dirname, '../../')
+ const DATA_ROOT = path.join(PROJECT_ROOT, 'src/tests/data/')
+
+ let client: DebugClient
+
+ setup(() => {
+ client = new DebugClient('node', DEBUG_ADAPTER, 'dfdl')
+ return client.start()
+ })
+
+ teardown(() => client.stop())
+
+ suite('basic', () => {
+ test('unknown request should produce error', (done) => {
+ client
+ .send('illegal_request')
+ .then(() => {
+ done(new Error('does not report error on unknown request'))
+ })
+ .catch(() => {
+ done()
+ })
+ })
+ })
+
+ suite('initialize', () => {
+ test('should return supported features', () => {
+ return client.initializeRequest().then((response) => {
+ response.body = response.body || {}
+ assert.strictEqual(response.body.supportsConfigurationDoneRequest, true)
+ })
+ })
+
+ test("should produce error for invalid 'pathFormat'", (done) => {
+ client
+ .initializeRequest({
+ adapterID: 'dfdl',
+ linesStartAt1: true,
+ columnsStartAt1: true,
+ pathFormat: 'url',
+ })
+ .then((response) => {
+ done(
+ new Error("does not report error on invalid 'pathFormat' attribute")
+ )
+ })
+ .catch((err) => {
+ // error expected
+ done()
+ })
+ })
+ })
+
+ suite('launch', () => {
+ test('should run program to the end', () => {
+ const PROGRAM = path.join(DATA_ROOT, 'DFDLGeneralFormat.dfdl.xsd')
+
+ return Promise.all([
+ client.configurationSequence(),
+ client.launch({ program: PROGRAM }),
+ client.waitForEvent('terminated'),
+ ])
+ })
Review comment:
Is it possible to add a test that parses data using the schema? And uses the debugger to step through things, verify infoset output, etc.? That can be done later, but I assume that's the kind of tests that we'll eventually want to make sure things work as expected?
--
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: commits-unsubscribe@daffodil.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org