Skip to content

Remove continue-on-error so the default applies#2303

Merged
Adamantios merged 21 commits intomainfrom
chore/continue-on-error
Feb 18, 2025
Merged

Remove continue-on-error so the default applies#2303
Adamantios merged 21 commits intomainfrom
chore/continue-on-error

Conversation

@Adamantios
Copy link
Collaborator

Proposed changes

The default value for the continue-on-error flag is false. Because the flag was set to true for the unit tests, a green status was being reported despite them failing due to an infinite loop introduced in the dev mode in #2221. As a result, the failure went unnoticed for 5 months, which eventually led to additional bugs in the CLI, since its tests appeared to pass.

This issue was first mentioned in #2292 (comment).

Fixes

n/a

Types of changes

What types of changes does your code introduce? (A breaking change is a fix or feature that would cause existing functionality and APIs to not work as expected.)
Put an x in the box that applies

  • Non-breaking fix (non-breaking change which fixes an issue)
  • Breaking fix (breaking change which fixes an issue)
  • Non-breaking feature (non-breaking change which adds functionality)
  • Breaking feature (breaking change which adds functionality)
  • Refactor (non-breaking change which changes implementation)
  • Messy (mixture of the above - requires explanation!)

Checklist

Put an x in the boxes that apply.

  • I have read the CONTRIBUTING doc
  • I am making a pull request against the main branch (left side). Also you should start your branch off our main.
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have locally run services that could be impacted and they do not present failures derived from my changes
  • Public-facing documentation has been updated with the changes affected by this PR. Even if the provided contents are not in their final form, all significant information must be included.
  • Any backwards-incompatible/breaking change has been clearly documented in the upgrading document.

Further comments

n/a

The default value for the `continue-on-error` flag is `false`. Because the flag was set to `true` for the unit tests, a green status was being reported despite them failing due to an infinite loop introduced in the dev mode in #2221. As a result, the failure went unnoticed for 5 months, which eventually led to additional bugs in the CLI, since its tests appeared to pass.
OjusWiZard
OjusWiZard previously approved these changes Feb 12, 2025
Copy link
Member

@OjusWiZard OjusWiZard left a comment

Choose a reason for hiding this comment

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

image

This reverts commits 3cb05bc && cd3a205.
The deployment build process requires the directories.

This commit properly fixes the issue that was worked around in #2292 and rolled back in e214049.
@Adamantios Adamantios mentioned this pull request Feb 12, 2025
13 tasks
@Adamantios Adamantios mentioned this pull request Feb 17, 2025
13 tasks
solarw
solarw previously approved these changes Feb 18, 2025
Copy link
Collaborator

@solarw solarw left a comment

Choose a reason for hiding this comment

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

lgtm

Adamantios and others added 6 commits February 18, 2025 12:18
Fix the regressed packages' tests

[no ci]
Fix the regressed CLI tests

[no ci]
Fix `autonomy deploy build` command

[no ci]
- Replaced `while` loop with a `for`, using `itertools.chain` for cleaner iteration over path and parents (#2304 (comment)).
- Used `is_file()` instead of `exists()` to explicitly check for `packages.json`.
- Improved error message for clarity.
- Simplified path resolution by avoiding redundant `Path(path)`.
Fix infinite loop in dev mode

[no ci]
@Adamantios Adamantios dismissed stale reviews from solarw and OjusWiZard via bd50034 February 18, 2025 14:23
# Conflicts:
#	docs/package_list.md
#	packages/packages.json
#	packages/valory/agents/offend_slash/aea-config.yaml
#	packages/valory/agents/register_termination/aea-config.yaml
#	packages/valory/contracts/gnosis_safe/contract.yaml
#	packages/valory/skills/offend_slash_abci/skill.yaml
#	packages/valory/skills/register_termination_abci/skill.yaml
#	packages/valory/skills/slashing_abci/skill.yaml
#	packages/valory/skills/termination_abci/skill.yaml
#	packages/valory/skills/transaction_settlement_abci/skill.yaml
@codecov
Copy link

codecov bot commented Feb 18, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.12%. Comparing base (8bdce08) to head (d3a9de0).
Report is 235 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2303      +/-   ##
==========================================
- Coverage   94.40%   93.12%   -1.28%     
==========================================
  Files         260      225      -35     
  Lines       16110    16366     +256     
==========================================
+ Hits        15208    15241      +33     
- Misses        902     1125     +223     
Flag Coverage Δ
unittests 93.12% <100.00%> (-1.28%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Adamantios Adamantios merged commit a62c194 into main Feb 18, 2025
@Adamantios Adamantios deleted the chore/continue-on-error branch February 18, 2025 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants