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.

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

Dd 1689 dd manage deposit unit tests failure and skipped #9

Conversation

aliassheikh
Copy link
Contributor

@aliassheikh aliassheikh commented Oct 27, 2024

Fixes DD-1689 dd-manage-deposit unit tests: one failure and one skipped

Description of changes

  • Don´t skip the skipped unit test: should_pick_up_deleted_root()
  • fix failure unit test: should_pick_up_changed_properties()

How to test

MVNCI

Related PRs

(Add links)

Notify

@DANS-KNAW/core-systems

@aliassheikh aliassheikh requested a review from a team as a code owner October 27, 2024 23:10
Copy link

codecov bot commented Oct 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 33.74%. Comparing base (41cf471) to head (811adbe).
Report is 7 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master       #9      +/-   ##
============================================
- Coverage     35.37%   33.74%   -1.63%     
- Complexity       45       49       +4     
============================================
  Files            16       16              
  Lines           359      403      +44     
  Branches         28       35       +7     
============================================
+ Hits            127      136       +9     
- Misses          226      261      +35     
  Partials          6        6              

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

Copy link
Contributor

@jo-pol jo-pol left a comment

Choose a reason for hiding this comment

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

Please also remove the no longer used

/**
* Assume that a bug is not yet fixed. This allows to skip assertions while still showing the code covered by the test.
*
* @param message the message to display
*/
public void assumeNotYetFixed (String message) {
assumeTrue(false, message);
}

Copy link
Contributor

@janvanmansum janvanmansum left a comment

Choose a reason for hiding this comment

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

Note: needing waits in your unit tests in order for them to pass seems a bit brittle to me. I'll approve it anyway, as waiting longer will at least not make the situation worse in this case.

@janvanmansum janvanmansum merged commit d64e9d2 into DANS-KNAW:master Nov 6, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants