GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Changed ordering of tests for suites with context by jgebal · Pull Request #1037 · utPLSQL/utPLSQL

Conversation

jgebal commented Dec 29, 2019

Copy link

Copy Markdown

Member

Resolves #1036

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jgebal added this to the 3.1.10 milestone

Dec 29, 2019

jgebal added the bugfix label

Dec 29, 2019

jgebal requested a review from pesse

December 29, 2019 01:01

pesse suggested changes

Jan 2, 2020

View reviewed changes

pesse left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Test OK.
Tested on local develop-environment with APP-user on ut3. Changes in position of --%context and --%test annotations are correctly reflected in the output.
Test-package used:

create or replace package ut_context_test as

  -- %suite(Context Test)
  -- %suitepath(context)


  -- %context(Level 2)
    --%name(proj_same)

    -- %test(Test 2.1 )
    procedure test_2_1;

  -- %endcontext

  -- %context(Level 1)
    --%name(proj_diff)

    -- %context(Level 1.1)

      -- %test(Test 1.1.2)
      procedure test_1_1_2;
      -- %test(Test 1.1.1)
      procedure test_1_1_1;

    -- %endcontext

  -- %endcontext

end;
/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

source/core/ut_suite_cache_manager.pkb Outdated

		case when c.obj.self_type = 'UT_SUITE_CONTEXT' then
		( select max( x.line_no ) + 1
		from ut_suite_cache x
		where c.obj.object_owner = x.object_owner and c.obj.object_name = x.object_name and

pesse Jan 2, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Aligning the multiple criteria would improve readability, or?

where c.obj.object_owner = x.boject_owner
   and c.obj.object_name = x.object_name
   and x.path like c.obj.path || '.%'

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jgebal Jan 3, 2020

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Fixed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

source/core/ut_suite_cache_manager.pkb

		then substr(c.obj.path, 1, instr(c.obj.path, '.', -1) )
		else c.obj.path
		end, '.', chr(0)
		substr( c.obj.path, 1, instr( c.obj.path, lower(c.obj.object_name), -1 ) + length(c.obj.object_name) ),

pesse Jan 2, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I find this very hard to read/understand. I guess extracting to a helper function would be too much overhead - can we add a comment to explain the intention?
-- get path until object's name

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jgebal Jan 3, 2020

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It is documented properly now.
Thanks for this comment. It helps me a lot to see what is not obvious/clear.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jgebal added 2 commits

January 3, 2020 05:34


          Changed ordering of tests for suites with context

d02ac94

Resolves #1036


          Test name fix

0a4edaf

jgebal force-pushed the feature/test_ordering branch from 234be36 to 1da8720 Compare

January 3, 2020 05:34


          Added documenting comments for ordering of data from suite_cache.

326e055

Fixed SQL formatting.

jgebal force-pushed the feature/test_ordering branch from 1da8720 to 326e055 Compare

January 3, 2020 05:37

jgebal commented Jan 4, 2020

Copy link

Copy Markdown

Member Author

@pesse - Can you review and check if you're OK with the changes after requested fixes?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jgebal merged commit d9077a1 into develop

Jan 4, 2020

jgebal deleted the feature/test_ordering branch

January 4, 2020 12:27

Reviewers

1 more reviewer

pesse

pesse requested changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

bugfix

Projects

None yet

Milestone

v3.1.10

Development

Successfully merging this pull request may close these issues.

Odd default ordering of tests when using contexts

Uh oh!

There was an error while loading. Please reload this page.

2 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.