Skip to content
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

Jump to bottom

Map shortened scala test suite names to long names on Windows #10628

Merged
merged 7 commits into from
Aug 24, 2021
Merged

Map shortened scala test suite names to long names on Windows #10628

merged 7 commits into from
Aug 24, 2021

Conversation

aherrmann-da
Copy link
Contributor

@aherrmann-da aherrmann-da commented Aug 19, 2021 •
edited
Loading

Generates a JSON file on Windows builds mapping shortened Scala test labels to their long counterparts. An example can be seen here. And patches the Windows Bazel metrics before uploading them to ElasticSearch.

da_scala_test_suite generates one scala_test target per Scala source file in srcs. The name of the generated test target is based on the path of the source file. On Linux and MacOS this target name has the form //libs-scala/grpc-reverse-proxy:test_test_suite_src_test_scala_com_daml_grpc_ReverseProxySpec.scala on Windows it is shortened to the form //libs-scala/grpc-reverse-proxy:test_1. This differenence causes Windows tests to be reported separately from their Linux and MacOS counterparts in the Bazel Kibana dashboard. Using the mapping file generated with this PR we can replace the shortened names by longer names for reporting purposes.

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.

@aherrmann-da aherrmann-da force-pushed the scala-test-name branch 2 times, most recently from f73d7c0 to 282f4c0 Compare August 20, 2021 15:21
@aherrmann-da aherrmann-da force-pushed the scala-test-name branch 3 times, most recently from 8897d67 to 384fc83 Compare August 24, 2021 10:24
aherrmann-da
aherrmann-da commented Aug 24, 2021
View reviewed changes
build.ps1
@@ -69,6 +69,10 @@ bazel build //... `
bazel shutdown

if ($env:SKIP_TESTS -ceq "False") {
# Generate mapping from shortened scala-test names on Windows to long names on Linux and MacOS.
./ci/remap-scala-test-short-names.ps1 `
| Out-File -Encoding UTF8 -NoNewline scala-test-suite-name-map.json
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step takes about 4.5 seconds on a Windows ad-hoc machine. The resulting file is about 70kB large.

aherrmann-da
aherrmann-da commented Aug 24, 2021
View reviewed changes
infra/es_cluster.tf
@@ -852,6 +869,7 @@ for tar in $todo; do
ensure_index "$job" "$(index "$job" jobs)"
ensure_index "$job" "$(index "$job" events)"
tar --force-local -x -z -f "$(basename "$tar")"
patch "$job"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This step takes a little less than 8 seconds on my machine. The generated sed command is about 70kB in size.

@aherrmann
Generate short to long name mapping in aspect
02cc11e 
Maps shortened test names in da_scala_test_suite on Windows to their
long name on Linux and MacOS.

Names are shortened on Windows to avoid exceeding MAX_PATH.
@aherrmann
Script to generate scala test name mapping
b03609f
@aherrmann
Generate scala-test-suite-name-map.json on Windows
47f1db9 
changelog_begin
changelog_end
@aherrmann
Generate UTF-8 with Unix line endings
96d5c50 
Otherwise the file will be formatted using UTF-16 with CRLF line
endings, which confuses `jq` on Linux.
@aherrmann
Apply Scala test name remapping before ES upload
66b5cb8
@aherrmann
Pipe bazel output into intermediate file
2de3a2d 
Bazel writes the output of --experimental_show_artifacts to stderr
instead of stdout. In Powershell this means that these outputs are not
plain strings, but instead error objects. Simply redirecting these to
stdout and piping them into further processing will lead to
indeterministically missing items or indeterministically introduced
additional newlines which may break paths.

To work around this we extract the error message from error objects,
introduce appropriate newlines, and write the output to a temporary file
before further processing.

This solution is taken and adapted from
https://stackoverflow.com/a/48671797/841562
@aherrmann-da aherrmann-da marked this pull request as ready for review August 24, 2021 13:52
cocreature
cocreature approved these changes Aug 24, 2021
View reviewed changes
Copy link
Contributor

@cocreature cocreature left a comment

Choose a reason for hiding this comment

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

Nice work!

@aherrmann-da aherrmann-da merged commit 7b94b06 into main Aug 24, 2021
@aherrmann-da aherrmann-da deleted the scala-test-name branch August 24, 2021 15:03
@garyverhaegen-da
Copy link
Contributor

Deployed and seems to be working as expected. I have seen a log line indicating the new code has run:

[...] applying scala-test-suite-name-map.json

and the corresponding files have been successfully imported into ES.

azure-pipelines bot pushed a commit that referenced this pull request Aug 25, 2021
release 1.17.0-snapshot.20210824.7647.0.640fb683
b289daf 
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.

@SamirTalwar-DA is in charge of this release.

Commit log:
```
640fb68 Make Index DB enable multiple party additions [DPP-546] (#10623)
b22de68 LF: Contract ID suffix check in Preprocessor (#10642)
7b94b06 Map shortened scala test suite names to long names on Windows (#10628)
6e4a24c participant-state: Generate correct gRPC error codes by v2 `WriteService` [KVL-1081] (#10656)
663781a Update curl 7.73.0 --> 7.78.0 (#10655)
a471f15 Dpp-558 Fix startexclusive queries on oracle (#10649)
e99254f Augment `completion.proto` with deduplication-related info [KVL-1057] (#10619)
a00608c participant-integration-api: Accommodate changes to max dedup time. (#10650)
29c546c [Divulgence pruning] Added `prune_all_divulged_contracts` to PruneRequest [DPP-534] (#10635)
dea57ca In-memory fan-out optimizations (#10558)
77eb366 [JSON-API] key_hash field to speed up fetchByKey queries (#10631)
5001329 LF: Comparisons fail at runtime if comparing local vs global CIDs (#10630)
055be4b Disable deprecation warnings for data-dependencies (#10647)
c155935 clean-up gsg README (#10641)
8501832 DPP-468 StorageBackend tests (#10529)
4e08b47 update NOTICES file (#10645)
733590d ledger-api-health: Use the Scala health status values everywhere. (#10640)
5b837ec Ledger API: add `buf` checks [KVL-1045] (#10625)
f77cd0a participant-integration-api: Attempt to fix RecoveringIndexerSpec. (#10639)
9d7f60f participant-integration-api: Fix a flaky test. (#10637)
4a9331c Upgrade Nixpkgs [KVL-1045] (#10624)
01b6e89 update compat versions for 1.17.0-snapshot.20210817.7604.0.0c187853 (#10610)
b578b0e Reminder to put an empty line between subject and body (#10638)
c0fbad1 participant-integration-api: Remove `limitMaxCommandsInFlight`. (#10626)
b4af6d1 Canton testing: Mark one more DeeplyNestedValueIT test flaky (#10636)
e807f4a Upgrade to a newer canton version (post 0.27.0 snapshot version) (#10632)
c4513f2 Oracle append-only schema: enable contract id index on participant_events_xxxx tables (#10633)
3598e09 LF: Drop contract ID Freshness check (#10620)
37c999e ledger-on-sql: Increase the concurrency for conformance tests. (#10622)
46e8c7d DPP-460 Extract constant for event sequential IDs (#10564)
121047e DPP-460 Parameter storage consolidation (#10472)
569612a Drop broken symlink config (#10616)
6d0109f Support $ in daml-lf identifiers in the parser (#10609)
c38703e participant-integration-api: Store a status gRPC protobuf. [KVL-1005] (#10600)
0af5b49 make FinalReason a case class (#10614)
8dd136f bazel-tools: Replace `runner` with either `runner_with_port_check` or `runner_with_port_file`. (#10615)
19c3d28 Remove GenMissingString class because it is not used (#10608)
3227e86 Use the port file and dynamic port generation in client/server tests. (#10604)
fb19bcb fix gsg-trigger template (#10611)
b207702 rotate release duty after 1.17.0-snapshot.20210817.7604.0.0c187853 (#10606)
975a5fb Move DeduplicationPeriod to ledger-api-domain [KVL-1047] (#10590)
f8a1820 release 1.17.0-snapshot.20210817.7604.0.0c187853 (#10605)
64abf8a update NOTICES file (#10607)
```
Changelog:
```
[Integration Kit] Corrected gRPC error codes returned by v2 `WriteService` adaptor.
- [Ledger API Server] The API server manages a single command tracker
  per (application ID × submitters) pair. This tracker would read the
  current ledger configuration's maximum deduplication time on creation,
  but never updated it, leading to trackers that might inadvertently
  reject a submission when it should have been accepted. The tracker now
  reads the latest ledger configuration.
- Update schema version for http-json-api query store with new key_hash field
- Improved performance for fetchByKey query which now uses key_hash field
participant-state - move `DeduplicationPeriod` to ledger-api-domain
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
mergify bot pushed a commit that referenced this pull request Aug 25, 2021
@azure-pipelines
release 1.17.0-snapshot.20210824.7647.0.640fb683 (#10659)
430c1cc 
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.

@SamirTalwar-DA is in charge of this release.

Commit log:
```
640fb68 Make Index DB enable multiple party additions [DPP-546] (#10623)
b22de68 LF: Contract ID suffix check in Preprocessor (#10642)
7b94b06 Map shortened scala test suite names to long names on Windows (#10628)
6e4a24c participant-state: Generate correct gRPC error codes by v2 `WriteService` [KVL-1081] (#10656)
663781a Update curl 7.73.0 --> 7.78.0 (#10655)
a471f15 Dpp-558 Fix startexclusive queries on oracle (#10649)
e99254f Augment `completion.proto` with deduplication-related info [KVL-1057] (#10619)
a00608c participant-integration-api: Accommodate changes to max dedup time. (#10650)
29c546c [Divulgence pruning] Added `prune_all_divulged_contracts` to PruneRequest [DPP-534] (#10635)
dea57ca In-memory fan-out optimizations (#10558)
77eb366 [JSON-API] key_hash field to speed up fetchByKey queries (#10631)
5001329 LF: Comparisons fail at runtime if comparing local vs global CIDs (#10630)
055be4b Disable deprecation warnings for data-dependencies (#10647)
c155935 clean-up gsg README (#10641)
8501832 DPP-468 StorageBackend tests (#10529)
4e08b47 update NOTICES file (#10645)
733590d ledger-api-health: Use the Scala health status values everywhere. (#10640)
5b837ec Ledger API: add `buf` checks [KVL-1045] (#10625)
f77cd0a participant-integration-api: Attempt to fix RecoveringIndexerSpec. (#10639)
9d7f60f participant-integration-api: Fix a flaky test. (#10637)
4a9331c Upgrade Nixpkgs [KVL-1045] (#10624)
01b6e89 update compat versions for 1.17.0-snapshot.20210817.7604.0.0c187853 (#10610)
b578b0e Reminder to put an empty line between subject and body (#10638)
c0fbad1 participant-integration-api: Remove `limitMaxCommandsInFlight`. (#10626)
b4af6d1 Canton testing: Mark one more DeeplyNestedValueIT test flaky (#10636)
e807f4a Upgrade to a newer canton version (post 0.27.0 snapshot version) (#10632)
c4513f2 Oracle append-only schema: enable contract id index on participant_events_xxxx tables (#10633)
3598e09 LF: Drop contract ID Freshness check (#10620)
37c999e ledger-on-sql: Increase the concurrency for conformance tests. (#10622)
46e8c7d DPP-460 Extract constant for event sequential IDs (#10564)
121047e DPP-460 Parameter storage consolidation (#10472)
569612a Drop broken symlink config (#10616)
6d0109f Support $ in daml-lf identifiers in the parser (#10609)
c38703e participant-integration-api: Store a status gRPC protobuf. [KVL-1005] (#10600)
0af5b49 make FinalReason a case class (#10614)
8dd136f bazel-tools: Replace `runner` with either `runner_with_port_check` or `runner_with_port_file`. (#10615)
19c3d28 Remove GenMissingString class because it is not used (#10608)
3227e86 Use the port file and dynamic port generation in client/server tests. (#10604)
fb19bcb fix gsg-trigger template (#10611)
b207702 rotate release duty after 1.17.0-snapshot.20210817.7604.0.0c187853 (#10606)
975a5fb Move DeduplicationPeriod to ledger-api-domain [KVL-1047] (#10590)
f8a1820 release 1.17.0-snapshot.20210817.7604.0.0c187853 (#10605)
64abf8a update NOTICES file (#10607)
```
Changelog:
```
[Integration Kit] Corrected gRPC error codes returned by v2 `WriteService` adaptor.
- [Ledger API Server] The API server manages a single command tracker
  per (application ID × submitters) pair. This tracker would read the
  current ledger configuration's maximum deduplication time on creation,
  but never updated it, leading to trackers that might inadvertently
  reject a submission when it should have been accepted. The tracker now
  reads the latest ledger configuration.
- Update schema version for http-json-api query store with new key_hash field
- Improved performance for fetchByKey query which now uses key_hash field
participant-state - move `DeduplicationPeriod` to ledger-api-domain
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garyverhaegen-da garyverhaegen-da garyverhaegen-da approved these changes

@cocreature cocreature cocreature approved these changes

@stefanobaghino-da stefanobaghino-da Awaiting requested review from stefanobaghino-da

Assignees
No one assigned
Labels
Standard-Change
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aherrmann-da @garyverhaegen-da @cocreature @aherrmann