Enriched warning message on implicit/explicit commit #371
Conversation
1 similar comment
|
I was thinking of following approach: This way we can identify exactly where the savepoint was broken and provide a proper error/failure/warning message. I'll ad the same comment to #364. |
|
Changes Unknown when pulling 6a2eefb on Pazus:issue-364 into ** on utPLSQL:develop**. |
1 similar comment
|
Changes Unknown when pulling 6a2eefb on Pazus:issue-364 into ** on utPLSQL:develop**. |
1 similar comment
1 similar comment
2 similar comments
|
@jgebal if a test has commit, I can find the place of the commit in Do you think we need it? Also several commits might occure, do we need to list them all? |
1 similar comment
source/core/types/ut_executable.tpb
Outdated
| --listener - after call to executable | ||
| a_listener.fire_after_event(self.associated_event_name, a_item); | ||
|
|
||
| if l_start_transaction_id != dbms_transaction.local_transaction_id(true) then |
There was a problem hiding this comment.
Why do you create a new transaction here? I'd compare with null.
There was a problem hiding this comment.
If beforetest commits and test commits, we cannot "see" that the test committed as there was no transaction (it was committed at the time we checked).
There was a problem hiding this comment.
But then function will return null, we should check for null.
if l_start_transaction_id != dbms_transaction.local_transaction_id Or dbms_transaction.local_transaction_id is null
There was a problem hiding this comment.
I mean, we need to create transaction at the start.
You're right, we don't need to create it at the end, but do an additional null check.
If you think it should be changed, so that we don't create transaction after execution, can you update the PR?
I'm ok with both solutions.
source/core/types/ut_suite_item.tps
Outdated
| member procedure init( | ||
| self in out nocopy ut_suite_item, a_object_owner varchar2, a_object_name varchar2, a_name varchar2, | ||
| a_description varchar2, a_path varchar2, a_rollback_type integer, a_disabled_flag boolean), | ||
| member procedure set_disabled_flag(self in out nocopy ut_suite_item, a_disabled_flag boolean), | ||
| member function get_disabled_flag return boolean, | ||
| member function create_savepoint_if_needed return varchar2, | ||
| member procedure rollback_to_savepoint(self in out nocopy ut_suite_item, a_savepoint varchar2), | ||
| member function get_transaction_invalidators return ut_varchar2_list, | ||
| final member procedure add_transaction_invalidator(a_object_name varchar2), |
There was a problem hiding this comment.
Why is it final? Don't think the restriction is needed
| @@ -78,7 +78,7 @@ create or replace package body ut_annotations as | |||
| ,'%(' || c_rgexp_identifier || ')' | |||
| ,modifier => 'i' | |||
| ,subexpression => 1)); | |||
| l_annotation_params_str := trim(regexp_substr(l_annotation_str, '\((.*?)\)$', subexpression => 1)); | |||
| l_annotation_params_str := trim(regexp_substr(l_annotation_str, '\((.*?)\)\s*$', subexpression => 1)); | |||
There was a problem hiding this comment.
Could go as a separate PR for traceability
1 similar comment
|
@Pazus |
Processed check iа commit occured without starting new trancation
Resolves #364