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

Truncate errors at 256 characters, fix flaky test. #556

Merged
merged 3 commits into from
Sep 5, 2024
Merged

Truncate errors at 256 characters, fix flaky test. #556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f7344c2
Truncate errors at 256 characters.
dblock Sep 5, 2024
5c3739f
Get shard node before attempting to relocate it.
dblock Sep 5, 2024
6a0f9c4
Force the shard on node1.
dblock Sep 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,13 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Renamed `main` release tag to `main-latest` ([#321](https://github.com/opensearch-project/opensearch-api-specification/pull/321))
- Replaced usages of `Opensearch` with `OpenSearch` ([#335](https://github.com/opensearch-project/opensearch-api-specification/pull/335))
- Prevented merger tool from printing warnings when used by tester tool ([#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359))
- Replaced the deprecated fs.rmdirSync with fs.rmSync ([#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359))
- Tester tool now provides better context for non-2XX responses when --verbose is used ([#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359))
- Lock testing for next release of OpenSearch to a specific SHA ([#431](https://github.com/opensearch-project/opensearch-api-specification/pull/431))
- Replace nullable with null type ([#436](https://github.com/opensearch-project/opensearch-api-specification/pull/436))
- Replaced the deprecated `fs.rmdirSync` with `fs.rmSync` ([#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359))
- Added better context for non-2XX responses when `--verbose` is used with the tester tool ([#359](https://github.com/opensearch-project/opensearch-api-specification/pull/359))
- Locked testing for next release of OpenSearch to a specific SHA ([#431](https://github.com/opensearch-project/opensearch-api-specification/pull/431))
- Replaced nullable with `null` type ([#436](https://github.com/opensearch-project/opensearch-api-specification/pull/436))
- Split test suite ([#472])(https://github.com/opensearch-project/opensearch-api-specification/pull/472)
- Changed `WriteResponseBase`'s `_primary_term`, `_seq_no` & `_version` to have `int64` format ([#530](https://github.com/opensearch-project/opensearch-api-specification/pull/530))
- Adjust indices, shards cat API to test against unassigned indices ([#551](https://github.com/opensearch-project/opensearch-api-specification/pull/551))
- Adjusted indices, shards cat API to test against unassigned indices ([#551](https://github.com/opensearch-project/opensearch-api-specification/pull/551))
- Rename `Bytes` component to `StorageType` ([#552](https://github.com/opensearch-project/opensearch-api-specification/pull/552))
- Rename `ByteSize` to `StorageSize` ([#552](https://github.com/opensearch-project/opensearch-api-specification/pull/552))

Expand Down Expand Up @@ -143,7 +143,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Fixed `knn` query specification ([#538](https://github.com/opensearch-project/opensearch-api-specification/pull/538))
- Fixed content-type for `/hot_threads` ([#543](https://github.com/opensearch-project/opensearch-api-specification/pull/543))
- Fixed `/_cluster/settings` returning flat results ([#545](https://github.com/opensearch-project/opensearch-api-specification/pull/545))
- Fixed missing fields in cat API ([#551](https://github.com/opensearch-project/opensearch-api-specification/pull/551))
- Fixed missing fields in `_cat` API ([#551](https://github.com/opensearch-project/opensearch-api-specification/pull/551))

### Security

Expand Down
15 changes: 15 additions & 0 deletions tests/routing/cluster/reroute.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,18 @@ prologues:
- {director: Bennett Miller, title: Moneyball, year: 2011}
- {create: {_index: movies}}
- {director: Nicolas Winding Refn, title: Drive, year: 1960}
# force the shard on node1 in case it was created on node2
- path: /_cluster/reroute
method: POST
request:
payload:
commands:
- move:
index: movies
shard: 0
from_node: opensearch-node2
to_node: opensearch-node1
status: [200, 400]
epilogues:
- path: /movies
method: DELETE
Expand Down Expand Up @@ -51,6 +63,9 @@ chapters:
shard: 0
from_node: opensearch-node1
to_node: opensearch-node2
retry:
Copy link
Member Author

Choose a reason for hiding this comment

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

Shard may be still relocating, retry.

count: 3
wait: 1000
response:
status: 200
payload:
Expand Down
5 changes: 3 additions & 2 deletions tools/src/tester/ResultLogger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@ export class ConsoleResultLogger implements ResultLogger {
}

#maybe_shorten_error_message(message: string | undefined): string | undefined {
if (message === undefined || message.length <= 128 || this._verbose) return message
const part = message.split(',')[0]
const cut_at = 256
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: This is better off as a configurable number in the constructor, esp when we refactor the test framework to be a standalone product.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll leave it as is for now, but we can expose it as an option when someone actually wants to change this value programmatically.

if (message === undefined || message.length <= cut_at || this._verbose) return message
const part = message.substring(0, cut_at)
return part + (part !== message ? ', ...' : '')
}

Expand Down
23 changes: 23 additions & 0 deletions tools/tests/tester/ResultLogger.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,29 @@ describe('ConsoleResultLogger', () => {
])
})

test('with a very long error message', () => {
logger.log({
result: Result.PASSED,
display_path: 'path',
full_path: 'full_path',
description: 'description',
message: "x".repeat(257),
chapters: [{
title: 'title',
overall: {
result: Result.PASSED
}
}],
epilogues: [],
prologues: []
})

const truncated_error = `(${"x".repeat(256)}, ...)`
expect(log.mock.calls).toEqual([
[`${ansi.green('PASSED ')} ${ansi.cyan(ansi.b('path'))} ${ansi.gray(truncated_error)}`]
])
})

describe('with warnings', () => {
const logger = new ConsoleResultLogger(tab_width, true)

Expand Down
Loading