test: test for platform independent dars #10535

ghost · 2021-08-10T13:52:52Z

We add a daily test to check that dars don't depend on the underlying
operating system where the dar is build.

CHANGELOG_BEGIN
CHANGELOG_END

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

ci/cron/daily-compat.yml

ci/platform-independence-windows.yml

ci/platform-independence.yml

compiler/damlc/tests/src/platform-independence-test.sh

ci/platform-independence.yml

ci/platform-independence-windows.yml

cocreature

Nice work!

cocreature · 2021-08-12T08:30:33Z

ci/platform-independence.yml

+  - bash: |
+      set -euo pipefail
+      eval "$(dev-env/bin/dade-assist)"
+      bazel build //compiler/damlc/tests:platform-independence.dalf


can we check the full DAR rather than just this DALF? If those are identical the dalfs are clearly identical as well and our DARs should be built deterministically (if not we should probably fix that)so I think there shouldn’t be extra-nondeterminism introduced here.

The DARs are not identical: they include source code for a test file that differs on line endings, as well a .hie files that somehow differ too ("binary files differ", as diff puts it; I did not investigate further).

ah makes sense, can we unzip and diff all DALFs at least? While referenced ones should be identical since there package ids should show up, I think it’s still worth being explicit and we cover unreferenced ones (like some of the stable dalfs).

ci/cron/daily-compat.yml

compiler/damlc/tests/PlatformIndependence.daml

ci/cron/daily-compat.yml

garyverhaegen-da · 2021-08-12T11:50:27Z

This may be a bit of a dumb/way-too-late question, but, is there any reason why this is a daily run rather than part of every PR? Adding a publish step to the existing build-everything Windows build shouldn't be that much of an overhead, and the checks on macOS and Linux will run in parallel and be fairly short.

cocreature · 2021-08-12T11:54:01Z

This may be a bit of a dumb/way-too-late question, but, is there any reason why this is a daily run rather than part of every PR? Adding a publish step to the existing build-everything Windows build shouldn't be that much of an overhead, and the checks on macOS and Linux will run in parallel and be fairly short.

I don’t really like the idea of adding something that is strictly sequential across to slow down builds even more. Yes assuming caching works out it’s probably only 2 minutes or so but that’s still non-neglible and it can be a lot worse if caching fails.
Given how rarely we change the code that could affect this, the daily run seemed like a better option.

garyverhaegen-da · 2021-08-12T12:02:02Z

This may be a bit of a dumb/way-too-late question, but, is there any reason why this is a daily run rather than part of every PR? Adding a publish step to the existing build-everything Windows build shouldn't be that much of an overhead, and the checks on macOS and Linux will run in parallel and be fairly short.

I don’t really like the idea of adding something that is strictly sequential across to slow down builds even more. Yes assuming caching works out it’s probably only 2 minutes or so but that’s still non-neglible and it can be a lot worse if caching fails.
Given how rarely we change the code that could affect this, the daily run seemed like a better option.

If you're worried about caching you could instead have all three builds publish their result, and then have a separate job that literally just downloads the three DARs and calls diff. That should be super fast: DARs are only ~240kb each, diff is present by default with no dev-env activation needed.

Would make this a lot easier to test and avoid the big blob of duplicated PowerShell code nobody is going to keep in sync.

cocreature · 2021-08-12T12:32:46Z

If you're worried about caching you could instead have all three builds publish their result, and then have a separate job that literally just downloads the three DARs and calls diff. That should be super fast: DARs are only ~240kb each, diff is present by default with no dev-env activation needed.

That’s a good point. I think I was imagining this being a bazel test but if it’s just a diff it’s not that bad. I don’t feel strongly about this so fine with changing it if @Robin-da also prefers this.

ci/cron/daily-compat.yml

garyverhaegen-da · 2021-08-16T16:07:03Z

/azp run

azure-pipelines · 2021-08-16T16:07:24Z

Azure Pipelines successfully started running 1 pipeline(s).

We add a daily test to check that dars don't depend on the underlying operating system where the dar is build. CHANGELOG_BEGIN CHANGELOG_END

@nickchapman-da

This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @nickchapman-da is in charge of this release. Commit log: ``` 0c18785 test: test for platform independent dars (#10535) 3822a8c ledger-api-test-tool: Split TransactionServiceIT into lots of suites. (#10585) 3cc9de5 Introduce dependency on Oracle image version (#10597) f8c0a35 rewrite trigger docs to follow gsg (#10509) aec601f Fix badly versioned empty tx (#10596) d924404 daml ledger export: export all parties (#10588) fb09b72 Improve divulgence warning message. (#10595) fd9d872 Upgrade to Oracle 19.12 (#10589) 386965c Upgrade Flyway to v7. (#10594) 654d2ee Bump url-parse to address dependabot alerts (#10593) 01b8e30 Simplify loading of logback file (#10592) 19bfdbe update NOTICES file (#10591) 5f7a369 [JSON-API] Perf gatling MultiUserQueryScenario (#10422) 56059f3 Upgrade path-parse to 1.0.7 (#10587) 5c80252 ledger-grpc: Fix the directory paths. [KVL-1005] (#10586) 9db5ccf Normalize transactions & values as a separate pass (#10524) 99e1d78 [Integrity Checker] Implemented pairwise Update normalizer (#10530) cc37bc3 Set ErrorInfo metadata flag for definite_answer [KVL-1005] (#10583) 147e8f2 Skip subject in changelog check (#10584) 244262d Handle imports for builtin types correctly in Daml export (#10575) 1fc281f ContractStorageBackend consolidation [DPP-462] (#10402) 0cbaa15 Codeowners: Notify committer maintainers on API changes. (#10557) 6492ceb JSON API: log ledger connection errors at every attempt (#10581) 933b58e Extract grpc statuses to separate module [KVL-1005] (#10582) 0ef8944 Use explicit types to track failures when submitting a request for execution [KVL-1005] (#10567) 4430f52 changelog_begin (#10579) dc4629f update NOTICES file (#10580) 8b0a0e7 update NOTICES file (#10578) 4b8b67a Upgrade Scalatest to v3.2.9. (#10576) 41e60f7 Upgrade to Scala 2.12.14 and 2.13.6. (#10573) c447898 Fix display of unhandled exceptions in the scenario service (#10572) 4e1a90d Enable --incompatible_remote_results_ignore_disk (#10571) d183ecc rotate release duty after 1.17.0-snapshot.20210811.7560.0.4f9de4ba (#10556) 76ecb44 update compat versions for 1.17.0-snapshot.20210811.7565.0.f1a55aa4 (#10563) 86a03fa Bump bazel max jvm memory (#10569) 1cc136c update NOTICES file (#10565) c69880c ledger-api-test-tool: Enforce test naming standards. (#10562) ee34d0f Track command - use types for error handling instead of grpc statuses [KVL-1005] (#10503) 93c25f3 Release 1.17 snapshot (#10560) ``` Changelog: ``` - [Ledger API Test Tool] The ``TransactionServiceIT`` test suite has been split into many test suites. If you are including or excluding it, you will need to use the new test suite names, or you can use "TransactionService" as a prefix for all of them. If you are including or excluding individual tests, you will need to update your arguments with the new test suite. You can find the new test suite by running the test tool with the ``--list-all`` flag and looking for the test's short identifier. The short identifiers have not changed, with the exception of ``TXNoContractKey``, which has been renamed to ``CKNoContractKey`` and is now in the ``ContractKeysIT`` test suite. * [Daml export] You can now set the ``--all-parties`` option to generate a ledger export as seen by all known parties. ledger-api-client - Propagate definite_answer as metadata in the GRPC response for submit/submitAndWait [JSON API] Ledger connection errors are now logged at every attempt akka-bindings: `LedgerClientBinding.commands` now returns a flow of `Either[CompletionFailure, CompletionSuccess]` instead of `Completion` for clearer error handling. For backwards compatiblity the new return type can be turned back into a `Completion` using `CompletionResponse.toCompletion` ``` CHANGELOG_BEGIN CHANGELOG_END

@nickchapman-da

This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @nickchapman-da is in charge of this release. Commit log: ``` 0c18785 test: test for platform independent dars (#10535) 3822a8c ledger-api-test-tool: Split TransactionServiceIT into lots of suites. (#10585) 3cc9de5 Introduce dependency on Oracle image version (#10597) f8c0a35 rewrite trigger docs to follow gsg (#10509) aec601f Fix badly versioned empty tx (#10596) d924404 daml ledger export: export all parties (#10588) fb09b72 Improve divulgence warning message. (#10595) fd9d872 Upgrade to Oracle 19.12 (#10589) 386965c Upgrade Flyway to v7. (#10594) 654d2ee Bump url-parse to address dependabot alerts (#10593) 01b8e30 Simplify loading of logback file (#10592) 19bfdbe update NOTICES file (#10591) 5f7a369 [JSON-API] Perf gatling MultiUserQueryScenario (#10422) 56059f3 Upgrade path-parse to 1.0.7 (#10587) 5c80252 ledger-grpc: Fix the directory paths. [KVL-1005] (#10586) 9db5ccf Normalize transactions & values as a separate pass (#10524) 99e1d78 [Integrity Checker] Implemented pairwise Update normalizer (#10530) cc37bc3 Set ErrorInfo metadata flag for definite_answer [KVL-1005] (#10583) 147e8f2 Skip subject in changelog check (#10584) 244262d Handle imports for builtin types correctly in Daml export (#10575) 1fc281f ContractStorageBackend consolidation [DPP-462] (#10402) 0cbaa15 Codeowners: Notify committer maintainers on API changes. (#10557) 6492ceb JSON API: log ledger connection errors at every attempt (#10581) 933b58e Extract grpc statuses to separate module [KVL-1005] (#10582) 0ef8944 Use explicit types to track failures when submitting a request for execution [KVL-1005] (#10567) 4430f52 changelog_begin (#10579) dc4629f update NOTICES file (#10580) 8b0a0e7 update NOTICES file (#10578) 4b8b67a Upgrade Scalatest to v3.2.9. (#10576) 41e60f7 Upgrade to Scala 2.12.14 and 2.13.6. (#10573) c447898 Fix display of unhandled exceptions in the scenario service (#10572) 4e1a90d Enable --incompatible_remote_results_ignore_disk (#10571) d183ecc rotate release duty after 1.17.0-snapshot.20210811.7560.0.4f9de4ba (#10556) 76ecb44 update compat versions for 1.17.0-snapshot.20210811.7565.0.f1a55aa4 (#10563) 86a03fa Bump bazel max jvm memory (#10569) 1cc136c update NOTICES file (#10565) c69880c ledger-api-test-tool: Enforce test naming standards. (#10562) ee34d0f Track command - use types for error handling instead of grpc statuses [KVL-1005] (#10503) 93c25f3 Release 1.17 snapshot (#10560) ``` Changelog: ``` - [Ledger API Test Tool] The ``TransactionServiceIT`` test suite has been split into many test suites. If you are including or excluding it, you will need to use the new test suite names, or you can use "TransactionService" as a prefix for all of them. If you are including or excluding individual tests, you will need to update your arguments with the new test suite. You can find the new test suite by running the test tool with the ``--list-all`` flag and looking for the test's short identifier. The short identifiers have not changed, with the exception of ``TXNoContractKey``, which has been renamed to ``CKNoContractKey`` and is now in the ``ContractKeysIT`` test suite. * [Daml export] You can now set the ``--all-parties`` option to generate a ledger export as seen by all known parties. ledger-api-client - Propagate definite_answer as metadata in the GRPC response for submit/submitAndWait [JSON API] Ledger connection errors are now logged at every attempt akka-bindings: `LedgerClientBinding.commands` now returns a flow of `Either[CompletionFailure, CompletionSuccess]` instead of `Completion` for clearer error handling. For backwards compatiblity the new return type can be turned back into a `Completion` using `CompletionResponse.toCompletion` ``` CHANGELOG_BEGIN CHANGELOG_END Co-authored-by: Azure Pipelines DAML Build <support@digitalasset.com>

We add a CI test to check that dars don't depend on the underlying operating system where the dar is build. CHANGELOG_BEGIN CHANGELOG_END

@nickchapman-da

This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @nickchapman-da is in charge of this release. Commit log: ``` 0c18785 test: test for platform independent dars (#10535) 3822a8c ledger-api-test-tool: Split TransactionServiceIT into lots of suites. (#10585) 3cc9de5 Introduce dependency on Oracle image version (#10597) f8c0a35 rewrite trigger docs to follow gsg (#10509) aec601f Fix badly versioned empty tx (#10596) d924404 daml ledger export: export all parties (#10588) fb09b72 Improve divulgence warning message. (#10595) fd9d872 Upgrade to Oracle 19.12 (#10589) 386965c Upgrade Flyway to v7. (#10594) 654d2ee Bump url-parse to address dependabot alerts (#10593) 01b8e30 Simplify loading of logback file (#10592) 19bfdbe update NOTICES file (#10591) 5f7a369 [JSON-API] Perf gatling MultiUserQueryScenario (#10422) 56059f3 Upgrade path-parse to 1.0.7 (#10587) 5c80252 ledger-grpc: Fix the directory paths. [KVL-1005] (#10586) 9db5ccf Normalize transactions & values as a separate pass (#10524) 99e1d78 [Integrity Checker] Implemented pairwise Update normalizer (#10530) cc37bc3 Set ErrorInfo metadata flag for definite_answer [KVL-1005] (#10583) 147e8f2 Skip subject in changelog check (#10584) 244262d Handle imports for builtin types correctly in Daml export (#10575) 1fc281f ContractStorageBackend consolidation [DPP-462] (#10402) 0cbaa15 Codeowners: Notify committer maintainers on API changes. (#10557) 6492ceb JSON API: log ledger connection errors at every attempt (#10581) 933b58e Extract grpc statuses to separate module [KVL-1005] (#10582) 0ef8944 Use explicit types to track failures when submitting a request for execution [KVL-1005] (#10567) 4430f52 changelog_begin (#10579) dc4629f update NOTICES file (#10580) 8b0a0e7 update NOTICES file (#10578) 4b8b67a Upgrade Scalatest to v3.2.9. (#10576) 41e60f7 Upgrade to Scala 2.12.14 and 2.13.6. (#10573) c447898 Fix display of unhandled exceptions in the scenario service (#10572) 4e1a90d Enable --incompatible_remote_results_ignore_disk (#10571) d183ecc rotate release duty after 1.17.0-snapshot.20210811.7560.0.4f9de4ba (#10556) 76ecb44 update compat versions for 1.17.0-snapshot.20210811.7565.0.f1a55aa4 (#10563) 86a03fa Bump bazel max jvm memory (#10569) 1cc136c update NOTICES file (#10565) c69880c ledger-api-test-tool: Enforce test naming standards. (#10562) ee34d0f Track command - use types for error handling instead of grpc statuses [KVL-1005] (#10503) 93c25f3 Release 1.17 snapshot (#10560) ``` Changelog: ``` - [Ledger API Test Tool] The ``TransactionServiceIT`` test suite has been split into many test suites. If you are including or excluding it, you will need to use the new test suite names, or you can use "TransactionService" as a prefix for all of them. If you are including or excluding individual tests, you will need to update your arguments with the new test suite. You can find the new test suite by running the test tool with the ``--list-all`` flag and looking for the test's short identifier. The short identifiers have not changed, with the exception of ``TXNoContractKey``, which has been renamed to ``CKNoContractKey`` and is now in the ``ContractKeysIT`` test suite. * [Daml export] You can now set the ``--all-parties`` option to generate a ledger export as seen by all known parties. ledger-api-client - Propagate definite_answer as metadata in the GRPC response for submit/submitAndWait [JSON API] Ledger connection errors are now logged at every attempt akka-bindings: `LedgerClientBinding.commands` now returns a flow of `Either[CompletionFailure, CompletionSuccess]` instead of `Completion` for clearer error handling. For backwards compatiblity the new return type can be turned back into a `Completion` using `CompletionResponse.toCompletion` ``` CHANGELOG_BEGIN CHANGELOG_END Co-authored-by: Azure Pipelines DAML Build <support@digitalasset.com>

ghost force-pushed the platform_independence_test branch 2 times, most recently from 1392a91 to 27c0140 Compare August 10, 2021 14:42