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

Add reporting for SQLDeveloper extension #795

Closed
jgebal opened this Issue Nov 23, 2018 · 18 comments

Comments

Projects
None yet
3 participants
@jgebal
Copy link
Member

jgebal commented Nov 23, 2018

As a SQLDeveloper user, I'd like to get pretty and interactive reports from my SQLDeveloper test-runs so that I can:

  • see progress of test execution
  • fold/unfold test suites and contexts
  • see duration of each test
  • see status of each test (executing/success/failed)
  • see failure report for every failed test
  • navigate to failed test line by clicking in failure report package link
  • rerun selected tests (suites)
  • see warnings for suites/tests
  • see dbms outputs captured by every test

In order to support all of those requirements, as separate reporter is needed.

@jgebal jgebal added the enhancement label Nov 23, 2018

@jgebal jgebal added this to the v3.1.4 milestone Nov 23, 2018

@jgebal jgebal self-assigned this Nov 23, 2018

@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Nov 23, 2018

The discussed idea was to use JUnit XML format to provide information at individual test level.

The trouble with that approach however is that:

  • JUnit format would provide outputs after a test finished execution
  • JUnit format holds some information at suite level, and so we would need to produce outputs as "individual suites" for every test
  • We would need to provide outputs for tests twice (before and after test was executed)

An alternative format could be used, but that would probably need to be a custom one.

@PhilippSalvisberg - con you provide some thoughts on this?

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Nov 23, 2018

I think we can simplify the problem.

From my point of view the new annotation API (available since 3.1.3) provides everything to get the scope of the units to be tested. Based on that the GUI can produce a tree based on the included test suites. So bullet point 2 is not relevant for this issue.

Technically I would run the tests in connection a and catch the output in connection b. It would be possible to parse the output in connection b and associate it to the nodes in the tree. It is ugly because I would rely on a current output format. Hence I think it must be another reporter with a well defined format. - I'd like to reuse an existing reporter if possible to reduce the maintenance effort in the utPLSQL core. I said in slack that I do not like the idea of parsing XML fragments. However, it's better than parsing DBMS_OUTPUT lines. I suggest to suspend working on this issue until I have done some tests with the existing reporters. Maybe I get what I want with one or a combination of some existing reporters. Chances are good that the functionality in the SQL Developer extensions can be built based on the existing core API.

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Nov 23, 2018

@jgebal You can assign this issue to me, if you want.

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

I've implemented a ut_realtime_reporter which is sufficient as basis for utPLSQL/utPLSQL-SQLDeveloper#6. Should work for #117 as well.

Links to the ut_realtime_reporter implementation:

Links to the tests for ut_realtime_reporter:

I've also extended all installation scripts to cover this new reporter and amended a test covering a list of all core reporters (test_get_reporters_list). I've tested it in 18c, but not in other database versions.

Here is a video showing a producer and consumer using the ut_realtime_reporter via SQL*Plus:

https://www.youtube.com/watch?v=80BMojrdCDk

I plan to submit a PR as soon as @jgebal and @pesse agree on the solution approach.

@mathewbutler

This comment has been minimized.

Copy link

mathewbutler commented Dec 29, 2018

Hi this just an observation - there’s XML format generation logic in here. Any thoughts about extracting this out and using a template engine to isolate the presentation logic? I say this not knowing if there is a decent template engine for plsql (velocity is a Java example). This might be another responsibility we could isolate across utPLSQL and crest a separate capability.

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

Yes, @mathewbutler there is potential to extract and isolate XML related logic (e.g in ut_utils.to_xml_number, ut_utils.get_xml_header, ut_compound_data_helper.get_column_info_xml, ut_coverage_cobertura_reporter, ut_junit_reporter, ut_sonar_test_reporter, ut_tfs_junit_reporter). However I suggest to open a dedicated issue for that. Otherwise we are ending up like Hal fixing a light bulb... 😉

Regarding template engines. There are two I know, which are running within an Oracle Database 11.2, 12.1, 12.2, 18.4. I used them within oddgen.

  • tePLSQL - lightweight, grammar based on Oracle PSP
  • FTLDB - a wrapper for FreeMaker (Java in the DB)

Both work quite nicely. Nonetheless I'd think twice before introducing additional dependencies. It increases complexity. You should be sure that the additional effort has some real benefits.

Some additional, related thoughts:

An utPLSQL reporter currently produces lines. A line is limited to 4000 bytes. What I would like to have is a mechanism to produce events instead of lines. A payload of such an event should not be limited to a number of bytes. The format of an event could be XML, JSON or something else. An option could be to implement an additional ut_output_buffer based on CLOB and enforce the ut_realtime_reporter to use a CLOB based output buffer. Then the ut_realtime_reporter would produce the following event types:

  • pre-run information (suites and tests to be executed, including the number of tests)
  • start test suite
  • start test
  • end test
  • end test suite
  • post-run information (basically indicates the end of a run)

Each of these events would produce a own and complete document (e.g. XML). An event would be technically transported within a single row. This would simplify things for the consuming clients.

Right now the ut_realtime_reporter produces lines with XML fragments and not really a complete XML document (even if the sum of all fragments make a valid XML document). The client needs to listen for start and end tags, construct based on the buffered data something like an "event document" and extract the necessary information. I'm sure we can deal with this situation in the client.

@jgebal and I agreed that we do the first iteration based on the current utPLSQL reporter capabilities. And then - based on the knowledge gained during the implementation of the client code - decide on the next steps. One reason more, to separate the request regarding the introduction of a template engine and/or XML library in utPLSQL.

@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Dec 29, 2018

@PhilippSalvisberg Here are my comments:

  1. CLOB output buffer would be better - the only concern I have is the performance of the buffer.
    Let's open a separate issue for this, implement it and compare performance of save/retrieve for CLOB vs VARCHAR2. If the performance degradation is within 10-30% it's fine. If it's 2-3 times slower - bummer.
  2. Templates - great idea - I agree that separate issue would be good to address it. We should probably do it systematically and look for all places that it would bring benefit in terms of simplicity and readability of the code.

tests

I love the way you've implemented the tests taking benefit for Oracle's build-in functionality for XML processing.

One comment about test descriptions.
Rather that describing what the test is checking: --%test(Check XML report structure), describe the tested code functionality: --%test(Builds appropriate XML report structure). That way, when executing the tests, we see a list of descriptions for functionalities (requirements) that are working.

Implementation

Each call to self.print_text is causing SQL statement (context switch) within autonomous transaction and performs a commit.
Current implementation of reporter will therefore cause excessive context switches and commits.
I would suggest implementing it in one of two ways:

  1. building the lines in each method and then printing the lines at the end with print_text_lines
  2. building the CLOB in each method and then printing it using print_clob

If you want to go with 2., the procedure ut_output_reporter_base.print_clob needs to be changed:
Currently it is:

    l_lines ut_varchar2_list;
  begin
    if a_clob is not null and dbms_lob.getlength(a_clob) > 0 then
      l_lines := ut_utils.clob_to_table(a_clob);
      for i in 1 .. l_lines.count loop
        self.print_text(l_lines(i));
      end loop;
    end if;
  end;

Will be far more performant to use print_text_lines, as it is doing bulk insert not a row-by-row:

  begin
    if a_clob is not null and dbms_lob.getlength(a_clob) > 0 then
      self.print_text_lines(
        ut_utils.convert_collection( ut_utils.clob_to_table( a_clob, 4000 ) ) 
      );
    end if;
  end;
@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Dec 29, 2018

Now that I think of it...
If you would go with print_clob approach, you could have reporter CLOB-ready.
The only outstanding thing would be to change the output_buffer to work on CLOBs.

@mathewbutler

This comment has been minimized.

Copy link

mathewbutler commented Dec 29, 2018

@PhilippSalvisberg Agree with your points;

  • there is value
  • need to consider cost/ benefit of introducing new dependencies
  • should be under a separate ticket

As framed, this was an observation, rather that a review comment. Really something I thought about skimming the code.

Love the idea of event based / Observer implementation for this. I’m not clear yet how this will be tested, and think this will also draw out some useful new capabilities for utPLSQL .

Also,like the idea of iterating on this as a way to inform the final design.

It’ll be interesting to watch this evolve.

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

Thank you, @jgebal for your input. I like the idea of describing the requirement instead of the test itself. However, I found it difficult to apply it. I probably need some more practice here... 😂.

I've changed the test specification as suggested. And I'm using now self.print_text_lines in a new flush_print_buffer method. This will reduce the calls to the output buffer to a minimum. However, it will not automatically make the ut_realtime_reporter compatible to a future event-driven output buffer. For that a change of the reporting structure is required. Multiple XML documents instead of one as outlined above. Implementing that is possible with flush_print_buffer being a CLOBor a ut_varchar2_rows. It's just a technical detail.

So the question is. Do we want to produce multiple XML documents now?

@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Dec 29, 2018

Would you like the output buffer to return more than just data column (as it is today).

Example:
Given the return type is CLOB for data attribute have additional attribute(column) describing event type so you can identify event type and make decission based on that before inspecting the data.

That could be of use for cli & html coverage reporter though instead of event we would serve item name/type.
This could help to address the issue of single huge html file when gathering covetage on huge project/schema.
Cli could split the output into multiple files (one per object).

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

I can imagine that an additional column such as event or type would be helpful for other cases such as the CLI. I think this would be a good move. However, the question remains. What do we do now with this reporter?

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

I suggest the following:

  1. rewrite the ut_realtime_reporter to produce XML documents per event type using the current line-based output buffer
  2. implementing a new document-based output buffer supporting event types and a clob payload
  3. adapt the ut_realtime_reporter to use the new document-based output buffer. Technically the XML documents will be transported in a single row and not splitted into separated lines anymore. There might be additional attributes for an output buffer entry, but the XML documents to be processed can have 100% the same structure.

This issue #755 should cover number 1 only. Based on that I can implement utPLSQL/utPLSQL-SQLDeveloper#6 as well. This would be a huge improvement, since right now this enhancement is not implementable using existing reporters in SQLDev.

For number 2 and 3 dedicated issues should be opened.

@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Dec 29, 2018

We can create a new output buffer with CLOB & item_type.
To make it backward-compatible with existing output buffer we could:

  • Implement new function to return refcursor of item_type & item_data (CLOB)
  • Implement pipelined function to return collection of objects (item_type & item_data)
  • Implement a backward-compatible function that would return only item-data as refcursor and pipelined. CLOB will get split by newline / 4000 chars (whatever comes first)

This shouldn't be much of a hustle. I can take that challenge.

@PhilippSalvisberg - To make you progress without the new reporter, use the ut_output_reporter_base.print_clob. While reworking output buffer I'll rework the API to accept CLOB.

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

Ok, I rewrite the ut_realtime_reporter to produce XML documents per item_type using clob.

@PhilippSalvisberg

This comment has been minimized.

Copy link
Member

PhilippSalvisberg commented Dec 29, 2018

@jgebal: Using pring_clobis not really better than using print_text_lines. Both are based on ut_varchar2_list. In fact it would lead to an additional conversion...

  member procedure print_clob(self in out nocopy ut_output_reporter_base, a_clob clob) is
    l_lines ut_varchar2_list;
  begin
    if a_clob is not null and dbms_lob.getlength(a_clob) > 0 then
      l_lines := ut_utils.clob_to_table(a_clob);
      for i in 1 .. l_lines.count loop
        self.print_text(l_lines(i));
      end loop;
    end if;
  end;
@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Dec 29, 2018

That will need to be fixed inside the method.

Also, can you work on utPLSQL repo directly? We could the collaborate on that feature branch. I'll add you to team.

@jgebal

This comment has been minimized.

Copy link
Member Author

jgebal commented Jan 1, 2019

Resolved by #809

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.