chore: Rollback after each test, due to premature commit via remove_user_permission
#29964
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…user_permission` - `remove_user_permission` in `test_warehouse_user` calls delete_doc that enqueues dynamic link deletion - Execution of background job eventually commits - While in the test suite it runs sequentially in the same thread and commits whatever was done until then - Which is why the rollback in `tearDownClass` is quite useless here - This premature commit causes many illegal transactions caught by `assertRaises` to be committed in the db - This creates faulty/dirty ledgers and breaks reports, as outiside the test suite this shouldn't/wouldn't happen - Rollback after each test, and for `test_warehouse_user` in particular, manually cancel transaction
Codecov Report
@@ Coverage Diff @@
## develop #29964 +/- ##
===========================================
+ Coverage 59.65% 59.68% +0.03%
===========================================
Files 1109 1109
Lines 69170 69167 -3
===========================================
+ Hits 41261 41284 +23
+ Misses 27909 27883 -26
|
ankush
pushed a commit
that referenced
this pull request
Feb 23, 2022
…user_permission` (backport #29964) (#29967) * chore: Rollback after each test, due to premature commit via `remove_user_permission` - `remove_user_permission` in `test_warehouse_user` calls delete_doc that enqueues dynamic link deletion - Execution of background job eventually commits - While in the test suite it runs sequentially in the same thread and commits whatever was done until then - Which is why the rollback in `tearDownClass` is quite useless here - This premature commit causes many illegal transactions caught by `assertRaises` to be committed in the db - This creates faulty/dirty ledgers and breaks reports, as outiside the test suite this shouldn't/wouldn't happen - Rollback after each test, and for `test_warehouse_user` in particular, manually cancel transaction (cherry picked from commit bf87437) # Conflicts: # erpnext/stock/doctype/stock_entry/test_stock_entry.py * test: Make Variant if absent in `test_variant_work_order`, keep test atomic (cherry picked from commit 5ff3705) * fix: Merge conflicts Co-authored-by: marination <maricadsouza221197@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue:
remove_user_permissionintest_warehouse_usercallsdelete_docthat enqueues dynamic link deletiontearDownClassis quite useless hereassertRaisesto be committed in the dbEg:
test_serial_no_reqdcreates a Serial No SLE without any Serial no in it, which is caught and assertedTestStockEntryruns, but due to the premature commit, it did notFix:
test_warehouse_userin particular, manually cancel transaction