From a829ade7918f979285d760881fd22b0f6dfd8272 Mon Sep 17 00:00:00 2001 From: Kevin Moore Date: Tue, 17 Jan 2023 10:51:52 -0800 Subject: [PATCH 1/4] Drop legacy null tests --- .../test/generated_message_test.dart | 95 +------------------ 1 file changed, 1 insertion(+), 94 deletions(-) diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart index 7d58091fc..1d3e09a4d 100644 --- a/protoc_plugin/test/generated_message_test.dart +++ b/protoc_plugin/test/generated_message_test.dart @@ -2,8 +2,6 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. -// @dart=2.11 - import 'package:protobuf/protobuf.dart'; import 'package:test/test.dart'; @@ -18,8 +16,6 @@ import '../out/protos/package3.pb.dart' as p3; import '../out/protos/reserved_names.pb.dart'; import '../out/protos/reserved_names_extension.pb.dart'; import '../out/protos/reserved_names_message.pb.dart'; -import '../out/protos/toplevel.pb.dart'; -import '../out/protos/toplevel_import.pb.dart' as t; import 'test_util.dart'; void main() { @@ -38,40 +34,6 @@ void main() { expect(value2.repeatedForeignMessage, value1.repeatedForeignMessage); }); - test('testSettersRejectNull', () { - var message = TestAllTypes(); - expect(() { - message.optionalString = null; - }, throwsArgumentError); - expect(() { - message.optionalBytes = null; - }, throwsArgumentError); - expect(() { - message.optionalNestedMessage = null; - }, throwsArgumentError); - expect(() { - message.optionalNestedMessage = null; - }, throwsArgumentError); - expect(() { - message.optionalNestedEnum = null; - }, throwsArgumentError); - expect(() { - message.repeatedString.add(null); - }, throwsArgumentError); - expect(() { - message.repeatedBytes.add(null); - }, throwsArgumentError); - expect(() { - message.repeatedNestedMessage.add(null); - }, throwsArgumentError); - expect(() { - message.repeatedNestedMessage.add(null); - }, throwsArgumentError); - expect(() { - message.repeatedNestedEnum.add(null); - }, throwsArgumentError); - }); - test('testDefaultMessageIsReadOnly', () { var message = TestAllTypes(); expect(message.optionalNestedMessage, @@ -107,34 +69,6 @@ void main() { assertRepeatedFieldsModified(message); }); - test('testRepeatedSettersRejectNull', () { - var message = TestAllTypes(); - - message.repeatedString.addAll(['one', 'two']); - expect(() { - message.repeatedString[1] = null; - }, throwsArgumentError); - - message.repeatedBytes.addAll(['one'.codeUnits, 'two'.codeUnits]); - expect(() { - message.repeatedBytes[1] = null; - }, throwsArgumentError); - - message.repeatedNestedMessage.addAll([ - TestAllTypes_NestedMessage()..bb = 318, - TestAllTypes_NestedMessage()..bb = 456 - ]); - expect(() { - message.repeatedNestedMessage[1] = null; - }, throwsArgumentError); - - message.repeatedNestedEnum - .addAll([TestAllTypes_NestedEnum.FOO, TestAllTypes_NestedEnum.BAR]); - expect(() { - message.repeatedNestedEnum[1] = null; - }, throwsArgumentError); - }); - test('testRepeatedAppend', () { var message = TestAllTypes() ..repeatedInt32.addAll([1, 2, 3, 4]) @@ -147,26 +81,6 @@ void main() { expect(message.repeatedForeignMessage[0].c, 12); }); - test('testRepeatedAppendRejectsNull', () { - var message = TestAllTypes(); - - expect(() { - message.repeatedForeignMessage.addAll([ForeignMessage()..c = 12, null]); - }, throwsArgumentError); - - expect(() { - message.repeatedForeignEnum.addAll([ForeignEnum.FOREIGN_BAZ, null]); - }, throwsArgumentError); - - expect(() { - message.repeatedString.addAll(['one', null]); - }, throwsArgumentError); - - expect(() { - message.repeatedBytes.addAll(['one'.codeUnits, null]); - }, throwsArgumentError); - }); - test('testSettingForeignMessage', () { var message = TestAllTypes() ..optionalForeignMessage = (ForeignMessage()..c = 123); @@ -263,8 +177,7 @@ void main() { // expect(MessageWithNoOuter.getDescriptor().getFile(), // MultipleFilesTestProto.getDescriptor()); - var tagNumber = message.getTagNumber('foreignEnum'); - expect(tagNumber, isNotNull); + var tagNumber = message.getTagNumber('foreignEnum') as int; expect(message.getField(tagNumber), EnumWithNoOuter.BAR); // Not currently supported in Dart protobuf. @@ -807,12 +720,6 @@ void main() { message.m3M = p3.M_M(); }); - test('testToplevel', () { - var message = t.M(); - message.t = T(); - t.SApi(null); - }); - test('to toDebugString', () { var value1 = TestAllTypes()..optionalString = 'test 123'; expect(value1.toString(), 'optionalString: test 123\n'); From 3d8694c0468f2532777e8c3e07ad5af030776545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Wed, 18 Jan 2023 11:32:21 +0100 Subject: [PATCH 2/4] Move legacy tests to another file, run the file with pubspec SDK --- .github/workflows/dart.yml | 65 +++++++++-- .../legacy_tests/generated_message_test.dart | 101 ++++++++++++++++++ protoc_plugin/mono_pkg.yaml | 6 ++ .../test/generated_message_test.dart | 2 +- tool/ci.sh | 6 +- 5 files changed, 172 insertions(+), 8 deletions(-) create mode 100644 protoc_plugin/legacy_tests/generated_message_test.dart diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index 2c5c24286..2b17bd544 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -240,7 +240,7 @@ jobs: uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protobuf;commands:test" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protobuf;commands:test_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protobuf os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0 @@ -277,7 +277,7 @@ jobs: uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protoc_plugin;commands:command_0-command_3-test" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protoc_plugin;commands:command_0-command_3-test_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protoc_plugin os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0 @@ -322,7 +322,7 @@ jobs: uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protobuf;commands:test" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protobuf;commands:test_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protobuf os:ubuntu-latest;pub-cache-hosted;sdk:dev @@ -359,7 +359,7 @@ jobs: uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin;commands:command_0-command_3-test" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin;commands:command_0-command_3-test_0" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin os:ubuntu-latest;pub-cache-hosted;sdk:dev @@ -404,7 +404,7 @@ jobs: uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:macos-latest;pub-cache-hosted;sdk:2.12.0;packages:protobuf;commands:test" + key: "os:macos-latest;pub-cache-hosted;sdk:2.12.0;packages:protobuf;commands:test_0" restore-keys: | os:macos-latest;pub-cache-hosted;sdk:2.12.0;packages:protobuf os:macos-latest;pub-cache-hosted;sdk:2.12.0 @@ -441,7 +441,7 @@ jobs: uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:macos-latest;pub-cache-hosted;sdk:dev;packages:protobuf;commands:test" + key: "os:macos-latest;pub-cache-hosted;sdk:dev;packages:protobuf;commands:test_0" restore-keys: | os:macos-latest;pub-cache-hosted;sdk:dev;packages:protobuf os:macos-latest;pub-cache-hosted;sdk:dev @@ -524,3 +524,56 @@ jobs: - job_004 - job_005 - job_006 + job_015: + name: "run_legacy_tests; linux; Dart 2.12.0; PKG: protoc_plugin; `./../tool/setup.sh`, `make protos`, `dart test legacy_tests/generated_message_test.dart`" + runs-on: ubuntu-latest + steps: + - name: Cache Pub hosted dependencies + uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d + with: + path: "~/.pub-cache/hosted" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protoc_plugin;commands:command_0-command_3-test_1" + restore-keys: | + os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0;packages:protoc_plugin + os:ubuntu-latest;pub-cache-hosted;sdk:2.12.0 + os:ubuntu-latest;pub-cache-hosted + os:ubuntu-latest + - name: Setup Dart SDK + uses: dart-lang/setup-dart@6a218f2413a3e78e9087f638a238f6b40893203d + with: + sdk: "2.12.0" + - id: checkout + name: Checkout repository + uses: actions/checkout@755da8c3cf115ac066823e79a1e1788f8940201b + - id: protoc_plugin_pub_upgrade + name: protoc_plugin; dart pub upgrade + run: dart pub upgrade + if: "always() && steps.checkout.conclusion == 'success'" + working-directory: protoc_plugin + - name: protoc_plugin; ./../tool/setup.sh + run: ./../tool/setup.sh + if: "always() && steps.protoc_plugin_pub_upgrade.conclusion == 'success'" + working-directory: protoc_plugin + - name: protoc_plugin; make protos + run: make protos + if: "always() && steps.protoc_plugin_pub_upgrade.conclusion == 'success'" + working-directory: protoc_plugin + - name: protoc_plugin; dart test legacy_tests/generated_message_test.dart + run: dart test legacy_tests/generated_message_test.dart + if: "always() && steps.protoc_plugin_pub_upgrade.conclusion == 'success'" + working-directory: protoc_plugin + needs: + - job_001 + - job_002 + - job_003 + - job_004 + - job_005 + - job_006 + - job_007 + - job_008 + - job_009 + - job_010 + - job_011 + - job_012 + - job_013 + - job_014 diff --git a/protoc_plugin/legacy_tests/generated_message_test.dart b/protoc_plugin/legacy_tests/generated_message_test.dart new file mode 100644 index 000000000..0103cf1e1 --- /dev/null +++ b/protoc_plugin/legacy_tests/generated_message_test.dart @@ -0,0 +1,101 @@ +// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +// @dart=2.11 + +import 'package:test/test.dart'; + +import '../out/protos/google/protobuf/unittest.pb.dart'; +import '../out/protos/toplevel.pb.dart'; +import '../out/protos/toplevel_import.pb.dart' as t; + +void main() { + test('testSettersRejectNull', () { + var message = TestAllTypes(); + expect(() { + message.optionalString = null; + }, throwsArgumentError); + expect(() { + message.optionalBytes = null; + }, throwsArgumentError); + expect(() { + message.optionalNestedMessage = null; + }, throwsArgumentError); + expect(() { + message.optionalNestedMessage = null; + }, throwsArgumentError); + expect(() { + message.optionalNestedEnum = null; + }, throwsArgumentError); + expect(() { + message.repeatedString.add(null); + }, throwsArgumentError); + expect(() { + message.repeatedBytes.add(null); + }, throwsArgumentError); + expect(() { + message.repeatedNestedMessage.add(null); + }, throwsArgumentError); + expect(() { + message.repeatedNestedMessage.add(null); + }, throwsArgumentError); + expect(() { + message.repeatedNestedEnum.add(null); + }, throwsArgumentError); + }); + + test('testRepeatedSettersRejectNull', () { + var message = TestAllTypes(); + + message.repeatedString.addAll(['one', 'two']); + expect(() { + message.repeatedString[1] = null; + }, throwsArgumentError); + + message.repeatedBytes.addAll(['one'.codeUnits, 'two'.codeUnits]); + expect(() { + message.repeatedBytes[1] = null; + }, throwsArgumentError); + + message.repeatedNestedMessage.addAll([ + TestAllTypes_NestedMessage()..bb = 318, + TestAllTypes_NestedMessage()..bb = 456 + ]); + expect(() { + message.repeatedNestedMessage[1] = null; + }, throwsArgumentError); + + message.repeatedNestedEnum + .addAll([TestAllTypes_NestedEnum.FOO, TestAllTypes_NestedEnum.BAR]); + expect(() { + message.repeatedNestedEnum[1] = null; + }, throwsArgumentError); + }); + + test('testRepeatedAppendRejectsNull', () { + var message = TestAllTypes(); + + expect(() { + message.repeatedForeignMessage.addAll([ForeignMessage()..c = 12, null]); + }, throwsArgumentError); + + expect(() { + message.repeatedForeignEnum.addAll([ForeignEnum.FOREIGN_BAZ, null]); + }, throwsArgumentError); + + expect(() { + message.repeatedString.addAll(['one', null]); + }, throwsArgumentError); + + expect(() { + message.repeatedBytes.addAll(['one'.codeUnits, null]); + }, throwsArgumentError); + }); + + test('testToplevel', () { + var message = t.M(); + message.t = T(); + t.SApi(null); + }); +} diff --git a/protoc_plugin/mono_pkg.yaml b/protoc_plugin/mono_pkg.yaml index d9625f261..9a1004841 100644 --- a/protoc_plugin/mono_pkg.yaml +++ b/protoc_plugin/mono_pkg.yaml @@ -13,3 +13,9 @@ stages: - command: make protos - test sdk: [pubspec, dev] + - run_legacy_tests: + - group: + - command: ./../tool/setup.sh + - command: make protos + - test: legacy_tests/generated_message_test.dart + sdk: [pubspec] diff --git a/protoc_plugin/test/generated_message_test.dart b/protoc_plugin/test/generated_message_test.dart index 1d3e09a4d..888c894a0 100644 --- a/protoc_plugin/test/generated_message_test.dart +++ b/protoc_plugin/test/generated_message_test.dart @@ -177,7 +177,7 @@ void main() { // expect(MessageWithNoOuter.getDescriptor().getFile(), // MultipleFilesTestProto.getDescriptor()); - var tagNumber = message.getTagNumber('foreignEnum') as int; + var tagNumber = message.getTagNumber('foreignEnum')!; expect(message.getField(tagNumber), EnumWithNoOuter.BAR); // Not currently supported in Dart protobuf. diff --git a/tool/ci.sh b/tool/ci.sh index a06e6c1c3..b2dda741a 100755 --- a/tool/ci.sh +++ b/tool/ci.sh @@ -99,10 +99,14 @@ for PKG in ${PKGS}; do echo 'dart format --output=none --set-exit-if-changed .' dart format --output=none --set-exit-if-changed . || EXIT_CODE=$? ;; - test) + test_0) echo 'dart test' dart test || EXIT_CODE=$? ;; + test_1) + echo 'dart test legacy_tests/generated_message_test.dart' + dart test legacy_tests/generated_message_test.dart || EXIT_CODE=$? + ;; *) echo -e "\033[31mUnknown TASK '${TASK}' - TERMINATING JOB\033[0m" exit 64 From d51a7e46b6d1f54b0bffac4dbcb0fec0498305ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Wed, 18 Jan 2023 11:50:14 +0100 Subject: [PATCH 3/4] Update new file copyright --- protoc_plugin/legacy_tests/generated_message_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protoc_plugin/legacy_tests/generated_message_test.dart b/protoc_plugin/legacy_tests/generated_message_test.dart index 0103cf1e1..db7ab4289 100644 --- a/protoc_plugin/legacy_tests/generated_message_test.dart +++ b/protoc_plugin/legacy_tests/generated_message_test.dart @@ -1,4 +1,4 @@ -// Copyright (c) 2013, the Dart project authors. Please see the AUTHORS file +// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. From 2244d264ae2010d63c1721cda45a8ea5f07a938d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=96mer=20Sinan=20A=C4=9Facan?= Date: Wed, 18 Jan 2023 11:57:16 +0100 Subject: [PATCH 4/4] Try specifying dirs to analyze --- .github/workflows/dart.yml | 8 ++++---- protoc_plugin/mono_pkg.yaml | 3 ++- tool/ci.sh | 4 ++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.github/workflows/dart.yml b/.github/workflows/dart.yml index 2b17bd544..c6760ab03 100644 --- a/.github/workflows/dart.yml +++ b/.github/workflows/dart.yml @@ -191,14 +191,14 @@ jobs: if: "always() && steps.protobuf_pub_upgrade.conclusion == 'success'" working-directory: protobuf job_006: - name: "format_analyze; linux; Dart dev; PKG: protoc_plugin; `dart format --output=none --set-exit-if-changed .`, `./../tool/setup.sh`, `make protos`, `dart analyze --fatal-infos`" + name: "format_analyze; linux; Dart dev; PKG: protoc_plugin; `dart format --output=none --set-exit-if-changed .`, `./../tool/setup.sh`, `make protos`, `dart analyze --fatal-infos bin lib test`" runs-on: ubuntu-latest steps: - name: Cache Pub hosted dependencies uses: actions/cache@4723a57e26efda3a62cbde1812113b730952852d with: path: "~/.pub-cache/hosted" - key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin;commands:format-command_0-command_3-analyze_0" + key: "os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin;commands:format-command_0-command_3-analyze_3" restore-keys: | os:ubuntu-latest;pub-cache-hosted;sdk:dev;packages:protoc_plugin os:ubuntu-latest;pub-cache-hosted;sdk:dev @@ -228,8 +228,8 @@ jobs: run: make protos if: "always() && steps.protoc_plugin_pub_upgrade.conclusion == 'success'" working-directory: protoc_plugin - - name: "protoc_plugin; dart analyze --fatal-infos" - run: dart analyze --fatal-infos + - name: "protoc_plugin; dart analyze --fatal-infos bin lib test" + run: dart analyze --fatal-infos bin lib test if: "always() && steps.protoc_plugin_pub_upgrade.conclusion == 'success'" working-directory: protoc_plugin job_007: diff --git a/protoc_plugin/mono_pkg.yaml b/protoc_plugin/mono_pkg.yaml index 9a1004841..f7cefe59b 100644 --- a/protoc_plugin/mono_pkg.yaml +++ b/protoc_plugin/mono_pkg.yaml @@ -5,7 +5,8 @@ stages: - format - command: ./../tool/setup.sh - command: make protos - - analyze: --fatal-infos + # Specify directores to exclude legacy_tests + - analyze: --fatal-infos bin lib test sdk: [dev] - run_tests: - group: diff --git a/tool/ci.sh b/tool/ci.sh index b2dda741a..be8477f47 100755 --- a/tool/ci.sh +++ b/tool/ci.sh @@ -79,6 +79,10 @@ for PKG in ${PKGS}; do echo 'dart analyze test' dart analyze test || EXIT_CODE=$? ;; + analyze_3) + echo 'dart analyze --fatal-infos bin lib test' + dart analyze --fatal-infos bin lib test || EXIT_CODE=$? + ;; command_0) echo './../tool/setup.sh' ./../tool/setup.sh || EXIT_CODE=$?