Skip to content

Fixed #600 Assert against non-sensible excluded_accounts - #601

Open
smoy wants to merge 2 commits into
mainfrom
fix/600-assertion-against-new-exclude_accounts
Open

Fixed #600 Assert against non-sensible excluded_accounts#601
smoy wants to merge 2 commits into
mainfrom
fix/600-assertion-against-new-exclude_accounts

Conversation

@smoy

@smoy smoy commented Aug 21, 2023

Copy link
Copy Markdown
Collaborator

What changed?

  • Assert against non-sensible excluded_accounts

Rationale

  • During import phase, we don't want to generate non-sensible excluded_accounts. If it's a new template, import logic does not know how to generate exclude_accounts. If there is an existing template, the best it can do if old template doesn't have exclude_account, it shouldn't add new excluded_accounts.

How was it tested?

If it was manually verified, list the instructions for your reviewers to follow.

  • Unit Tests
  • Functional Tests
  • Manually Verified

Relying existing test suite.

@smoy smoy self-assigned this Aug 21, 2023
castrapel
castrapel previously approved these changes Aug 21, 2023

@castrapel castrapel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@codecov

codecov Bot commented Aug 22, 2023

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 33.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.27%. Comparing base (e3f5f0c) to head (3c5ed76).
⚠️ Report is 186 commits behind head on main.

Files with missing lines Patch % Lines
iambic/core/template_generation.py 33.33% 8 Missing ⚠️

❌ Your patch check has failed because the patch coverage (33.33%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (73.27%) is below the target coverage (75.00%). You can increase the head coverage or adjust the target coverage.

❗ There is a different number of reports uploaded between BASE (e3f5f0c) and HEAD (3c5ed76). Click for more details.

HEAD has 2 uploads less than BASE
Flag BASE (e3f5f0c) HEAD (3c5ed76)
functional_tests 1 0
functional_tests_config_discovery 1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #601      +/-   ##
==========================================
- Coverage   83.01%   73.27%   -9.75%     
==========================================
  Files         105      105              
  Lines       12549    12560      +11     
==========================================
- Hits        10418     9203    -1215     
- Misses       2131     3357    +1226     
Flag Coverage Δ
functional_tests ?
functional_tests_config_discovery ?
unit_tests 73.27% <33.33%> (-0.05%) ⬇️

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

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

2 participants