Skip to content

feat(ARRR): dockerize zombie/pirate tests #2374

Merged
shamardy merged 31 commits intodevfrom
dockerize_pirate
Mar 31, 2025
Merged

feat(ARRR): dockerize zombie/pirate tests #2374
shamardy merged 31 commits intodevfrom
dockerize_pirate

Conversation

@borngraced
Copy link
Copy Markdown

@borngraced borngraced commented Feb 26, 2025

@borngraced borngraced self-assigned this Feb 26, 2025
@borngraced borngraced changed the title feat(ARRR): dockerize ZOMBIE tests feat(ARRR): dockerize zombie/pirate tests Feb 26, 2025
@shamardy
Copy link
Copy Markdown
Collaborator

shamardy commented Mar 3, 2025

@borngraced what is left to make this ready for review. @onur-ozkan @dimxy please ignore reviewing this until it's ready.

@borngraced
Copy link
Copy Markdown
Author

@borngraced what is left to make this ready for review. @onur-ozkan @dimxy please ignore reviewing this until it's ready.

Some minor adjustments to the zombie docker image still ongoing

@borngraced borngraced changed the base branch from dev to main March 4, 2025 22:06
this PR is slowly diverting from it's main purpose hance, I have remove
additional fixes that shouldn't have been done here...removed fixes will
be reimplemented `fix-zombie-tests` PR
@borngraced
Copy link
Copy Markdown
Author

this PR(#2374) is slowly diverting from it's main purpose. Hence, I have removed additional fixes that shouldn't have been done here...removed fixes will
be re-implemented in `improve shielded transactions change notes handling in swaps PR.

change the avg_blocktime for ZOMBIE to reflect docker image
`ac_blocktime` configuration
@dimxy
Copy link
Copy Markdown
Collaborator

dimxy commented Mar 11, 2025

Do I understand it right that we are going to solve the problems with concurrent zombie tests failure?
If so I am okay with this PR

dimxy
dimxy previously approved these changes Mar 17, 2025
Copy link
Copy Markdown
Collaborator

@dimxy dimxy left a comment

Choose a reason for hiding this comment

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

LGTM

(let's merge this PR and make sure zombie tests work in the PR #2331)

onur-ozkan
onur-ozkan previously approved these changes Mar 18, 2025
Copy link
Copy Markdown

@onur-ozkan onur-ozkan left a comment

Choose a reason for hiding this comment

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

LGTM with some minors

)
let mut watcher = self.z_fields.sync_state_connector.lock().await;
while let Some(sync) = watcher.sync_watcher.next().await {
if matches!(sync, SyncStatus::Finished { block_number: _, .. }) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
if matches!(sync, SyncStatus::Finished { block_number: _, .. }) {
if matches!(sync, SyncStatus::Finished { .. }) {

Comment on lines +502 to +508
let timeout = wait_until_ms(3000);
loop {
if conf_path.exists() {
break;
};
assert!(now_ms() < timeout, "Test timed out");
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggested change
let timeout = wait_until_ms(3000);
loop {
if conf_path.exists() {
break;
};
assert!(now_ms() < timeout, "Test timed out");
}
let timeout = wait_until_ms(3000);
while !conf_path.exists() {
assert!(now_ms() < timeout, "Test timed out");
}

Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

First iteration!

Comment on lines +132 to +133
pub const ZOMBIE_ASSET_DOCKER_IMAGE: &str = "docker.io/borngraced/zombietestrunner";
pub const ZOMBIE_ASSET_DOCKER_IMAGE_WITH_TAG: &str = "docker.io/borngraced/zombietestrunner:multiarch";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should move these to komodo platform in docker.io. This is not a blocker for merge as this is for testing but we should open a PR once moved.
c.c. @smk762

shamardy
shamardy previously approved these changes Mar 28, 2025
Copy link
Copy Markdown
Collaborator

@shamardy shamardy left a comment

Choose a reason for hiding this comment

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

LGTM! Please fix any unresolved comments from the previous review to merge this.

@shamardy shamardy merged commit 5797d2c into dev Mar 31, 2025
24 checks passed
@shamardy shamardy deleted the dockerize_pirate branch March 31, 2025 14:23
dimxy pushed a commit that referenced this pull request Apr 7, 2025
* dev:
  improvement(best-orders): return an rpc error when we can't find best orders (#2318)
  feat(utxo): support FIRO Spark verbose tx
  feat(ARRR): dockerize zombie/pirate tests  (#2374)
  improvement(event-streaming): move UnknownClient error to trace level (#2401)
  feat(tpu): implement 0 dexfee for kmd trading pairs (#2323)
  feat(db-arch): ctx functions and use of global db (#2378)
  feat(swap): add utxo/cosmos/ARRR pre-burn address output (#2112)
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request May 27, 2025
* lr-swap-wip: (45 commits)
  review (mariocynicys): fix iterators zipping, refactor 1inch url builder, add docs to cross prices data, remove extra coin decimals check
  added doc comments for LrData struct
  error msg improved
  fix tx value eth conversion
  eliminate from_api_error fn
  fix src_decimals var name
  improve bad api TokenInfo error messages
  improvement(best-orders): return an rpc error when we can't find best orders (GLEECBTC#2318)
  feat(utxo): support FIRO Spark verbose tx
  feat(ARRR): dockerize zombie/pirate tests  (GLEECBTC#2374)
  improvement(event-streaming): move UnknownClient error to trace level (GLEECBTC#2401)
  feat(tpu): implement 0 dexfee for kmd trading pairs (GLEECBTC#2323)
  feat(db-arch): ctx functions and use of global db (GLEECBTC#2378)
  feat(swap): add utxo/cosmos/ARRR pre-burn address output (GLEECBTC#2112)
  review (laruh): rename fn
  review (laruh): add fn to get contracts from LrData
  add TODO
  fix find best lr swap behaviour: skip lr provider error results (to use successful ones)
  refactor 1inch url builder
  fix 1inch result conversion test
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants