Skip to content

Conversation

@shwstppr
Copy link
Contributor

@shwstppr shwstppr commented Feb 29, 2024

Description

The SearchBuilder listDomainAndTypeAndNoTagSearch in ReservationDaoImpl.java is wrongly created by adding ACCOUNT_ID as part of the search param. Instead it should be DOMAIN_ID.
Additional fixes in test cases to:

  • add assert
  • add spacing

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • build/CI

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>
@shwstppr
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@shwstppr a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

Copy link
Member

@vishesh92 vishesh92 left a comment

Choose a reason for hiding this comment

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

clgtm

@codecov
Copy link

codecov bot commented Feb 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 30.98%. Comparing base (6f27b1f) to head (e8114e5).

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #8725      +/-   ##
============================================
+ Coverage     30.97%   30.98%   +0.01%     
- Complexity    33481    33505      +24     
============================================
  Files          5355     5355              
  Lines        375830   375830              
  Branches      54895    54895              
============================================
+ Hits         116414   116467      +53     
+ Misses       243974   243936      -38     
+ Partials      15442    15427      -15     
Flag Coverage Δ
simulator-marvin-tests 24.86% <100.00%> (+0.02%) ⬆️
uitests 4.36% <ø> (ø)
unit-tests 16.57% <0.00%> (-0.01%) ⬇️

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.

@shwstppr shwstppr changed the title engine-schema,test: fix domain searchbuilder, test fixes engine-schema,test: fix domain reservation searchbuilder, test fixes Feb 29, 2024
@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 8815

Copy link
Contributor

@sureshanaparti sureshanaparti left a comment

Choose a reason for hiding this comment

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

code lgtm

@sureshanaparti
Copy link
Contributor

@shwstppr can this be targeted for 4.19.1?

@shwstppr
Copy link
Contributor Author

@sureshanaparti I guess the change was introduced in #8362 therefore it needs to be in main only

@sureshanaparti
Copy link
Contributor

@sureshanaparti I guess the change was introduced in #8362 therefore it needs to be in main only

ok, thanks @shwstppr

@sureshanaparti
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@sureshanaparti a [SL] Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

Copy link
Contributor

@borisstoyanov borisstoyanov left a comment

Choose a reason for hiding this comment

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

LGTM

@yadvr yadvr added this to the 20.0.0 milestone Feb 29, 2024
@blueorangutan
Copy link

[SF] Trillian test result (tid-9348)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46908 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr8725-t9348-kvm-centos7.zip
Smoke tests completed. 129 look OK, 0 have errors, 0 did not run
Only failed and skipped tests results shown below:

Test Result Time (s) Test File

@shwstppr
Copy link
Contributor Author

shwstppr commented Mar 1, 2024

Merging based on approvals and test results

@shwstppr shwstppr merged commit 68a3e9e into apache:main Mar 1, 2024
@shwstppr shwstppr deleted the fix-domainreservation-dao branch March 1, 2024 11:53
dhslove pushed a commit to ablecloud-team/ablestack-cloud that referenced this pull request Mar 5, 2024
The SearchBuilder listDomainAndTypeAndNoTagSearch in ReservationDaoImpl.java is wrongly created by adding ACCOUNT_ID as part of the search param. Instead it should be DOMAIN_ID.
Additional fixes in test cases to:
- add assert
- add spacing
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.

6 participants