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

Initial import of CCL Unit Mocking. #7

Merged
merged 10 commits into from Feb 15, 2019
Merged

Initial import of CCL Unit Mocking. #7

merged 10 commits into from Feb 15, 2019

Conversation

cernertrevor
Copy link
Contributor

@cernertrevor
Copy link
Contributor Author

@morph-g

Copy link
Contributor

@feckertson feckertson left a comment

Choose a reason for hiding this comment

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

This is so cool Trevor.

https://wiki.cerner.com/display/public/1101discernHP/SELECT+INTO+TABLE+Table_Name+Using+Discern+Explorer

@param tableName
    The table to which the constraint will be added.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and elsewhere... "mocked table" or "name of mocked table" or something like "The name of the source table for the mock table that that will have the constraint added" (which seems awful wordy) instead of "the table".

As written it suggest the actual table will get the constraint OR that the consumer must figure out the actual name of the mock table and pass that.

"the name of the source table for the mock table to which the constraint will be applied" is a little less wordy and closer to the original statement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll go with your last suggestion "the name of the source table for the mock table..." since I don't want people to think that if I say "name of mocked table" that I mean the value returned from cclutDefineMockTable().

    A string of all constraints to be applied to the column.

Example:
call cclutAddMockConstraint("person", "name_last", "not null unique")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be pointed out at the beginning as a blanket statement that provided names are case-insensitive b/c the framework will ucase everything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'll add that. Hopefully most developers are already familiar with the fact that CCL/Oracle is case-insensitive, but I'll call it out just to be extra clear.

@param columnNames
    A pipe-delimited string of column names for the index.
@param isUnique
    1 to create a unique index; 0 to create a non-unique index
Copy link
Contributor

Choose a reason for hiding this comment

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

They are equivalent, but perhaps true and false rather than 1 and 0 in the doc and examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.


**cclutClearMockData(tableName = vc)**

Clears all data from the mock table. This is functionally similar to a truncate. tableName is required. The table
Copy link
Contributor

Choose a reason for hiding this comment

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

"...from a mock table...." or "...from a specified mock table...."
("the" doesn't make sense till after one has been specified at which point "the specified mock table" can be reduced to "the mock table")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. I'll go with "from a specified mock table".

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops. Only made the change in the .inc. Will fix this in the Markdown as well.

cclunit-framework-source/doc/CCLUTMOCKING.md Show resolved Hide resolved
* Scenario: Executes the program while replacing a subroutine with a mocked version *
************************************************************************************************************/
subroutine test_cclutExecuteProgramWithMocks_mock_subroutine(null)
call echo("~~~***~~~***test_cclutExecuteProgramWithMocks_mock_subroutine***~~~***~~~")
Copy link
Contributor

Choose a reason for hiding this comment

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

the goals of this test are covered by my suggested change to the previous test.

call cclutAsserti4Equal(CURREF, "test_cclutRemoveAllMocks_happy", size(cclut_mockTables->tables, 5), 0)
call cclutAsserti4Equal(CURREF, "test_cclutRemoveAllMocks_happy",
size(cclut_mockImplementations->implementations, 5), 0)
end ;test_cclutRemoveAllMocks_happy
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be interested in seeing a test that executes a misbehaving test case (one that has tests which create mock tables without cleaning them up) and confirms the tables are gone after the misbehaving test case has completed. To make it convincing the misbehaving test case should be an actual test case (i.e., an inc file in src/test/ccl vs. src/test/resources) whose teardownOnce confirms the mock tables persist at least till that point. Identifying a way to communicate the name of the mock table from the misbehaving test case back to the unit test that executes it will take some thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is a great idea. This also made me realize that I'm missing a use-case for the fail-fast scenario.

So, one, I need to add the removal there. Then there should be two corresponding tests; one for the fail-fast and one for the regular. I'm thinking for the fail-fast, since there's already a ut_cclut_ff.prg, that a misbehaving test case is created for that in src/test/resources, and then a new test is added to ut_cclut_ff that calls that and does the other things you mention (misbehaving checks it's there in teardownOnce and ut_cclut_ff checks that it's gone). Since it's just calling down, the misbehaving script could set the name of the mock table that it gets back on to a variable defined by the test script.

For the regular one, I'm thinking it would do a similar thing (using the same misbehaving .inc), but from ut_cclut_execute_test_case(and file) instead. However, in both these scenarios, the misbehaving test case is in src/test/resources (whereas you made it sound like you wanted it in src/test/ccl), so I just want to make sure that this approach would still be in line with your expectations. If not, can you provide some more insight into your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are two different things to test. (1) mocks get cleaned up after a test case completes, and (2) mocks do not cleaned up before a test case completes. For (1), it is perfectly natural to have a test in src/test/ccl execute a misbehaving test case from src/test/resources. However, the test case for (2) must live in src/test/ccl. All I am really suggesting is to use the test case for (2) as the misbehaving test case leveraged for (1).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, sure.

size(cclut_mockTables->tables[1].indexes, 5), 0)
call cclutAsserti4Equal(CURREF, "test_cclutDefineMockTable_existing_mock", cclut_mockTables->tables[1].isFinalized,
FALSE)

Copy link
Contributor

Choose a reason for hiding this comment

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

the asserts before here can be safely omitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

end ;test_cclutDefineMockTable_happy

/* test_cclutDefineMockTable_existing_mock ******************************************************************
* Scenario: Removes the existing mock and adds a new one when cclutDefineMockTable is called again *
Copy link
Contributor

Choose a reason for hiding this comment

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

Works the same even if the table is finalized, no? If so, there should be a similar test for that case. If not, the documentation should spell that. It should probably spell it out one way or the other.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes... thought I had a test for that, but no, must've forgotten that. I'll add one.

@@ -8,7 +8,7 @@ http://www.springframework.org/schema/context http://www.springframework.org/sch

<!-- ftp-util configuration -->
<bean factory-bean="ftpProductBuilder" factory-method="build" />
<bean id="ftpProductBuilder" class="com.cerner.ccl.testing.framework.internal.UserPassBuilderSpringProxy" p:password="#{systemProperties['ccl-hostPassword']}" p:username="#{systemProperties['ccl-hostUsername']}" p:serverAddress="${ccl-host}">
Copy link
Contributor

Choose a reason for hiding this comment

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

Please revert the changes to the cclut-framework-tests project.

Although they allow the build to work on devices which only have ccl-hostUsername and ccl-hostPassword values configured, they prevent the build from working on devices which only have ccl-hostCredentialsId set up which is how the continuous integration box used to deploy the framework code is set up.

A change that works for both configurations would be okay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, yeah, this was a mistake; I didn't mean to include this in the commit. It was just part of the initial troubleshooting I was doing.

Copy link
Contributor

@feckertson feckertson left a comment

Choose a reason for hiding this comment

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

I'm not sure if you are aware of PR9 which I think is the correct correction for the cclut_find_unit_tests issue. I tried to add you as a reviewer without luck and you are not listed as a repository watcher.

@cernertrevor
Copy link
Contributor Author

I know it's taken a little while due to some other obligations but the first batch of changes is now available. @feckertson There are still a few comments of yours that I'm waiting on a response from you before I implement, so once I have those, I'll make the appropriate changes and perform another push.

@feckertson
Copy link
Contributor

@cernertrevor I am not aware of any open discussions I have disagreement with.

call parser(cclutParserText)

;Special case if rowData is empty for single-column tables
if (cclutMockDataLength = 0 or (cclutMockDataLength = 1 and ichar(substring(1, 1, CCLUT_ROW_DATA)) = 0))
Copy link
Contributor

Choose a reason for hiding this comment

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

When I did it, I did not set the table attribute equal to value trim(" ") but to a VC variable that was previously set equal to trim(" "). Turning on rdbbind shows that CCL sends different values for those cases.

size(cclut_mockImplementations->implementations, 5), 0)
end ;test_cclutRemoveMockImplementation_happy

/* test_cclutRemoveMockImplementation_missing_originalName *****************************************************
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct, the documentation format [currently] only matters for the source. I simply didn't notice I hadn't been paying the format any attention till I got to this point. The different documentation style used for the tests is what brought it to my attention (I tend to use the same style everywhere). I did not notice the context (unit tests) for this particular documentation, but I also did not start scrutinizing the doc which is a job better done by inspecting the CDoc output.

Looking back, I do notice that some [source] documentation sets off parameter documentation with several non-breaking spaces while other [source] documentation does not. Also, when examples are provided, they are not set off with breaks or addition HTML formatting. It's unlikely a high-demand enhancement, but improvements to CDoc and the CDoc output generation would be a better way to handle some of this... adding an @example tag and the ability to better control the output format, for example.

;**********************************************************************************************************************************
/* test_cclutExecuteProgramWithMocks_happy ******************************************************************
* Scenario: Executes the program using mock tables and implementations that have been set up *
************************************************************************************************************/
Copy link
Contributor

Choose a reason for hiding this comment

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

Something along those lines would better describe what the test does than what is currently written.

/* test_cclutExecuteProgramWithMocks_namespace **************************************************************
* Scenario: Executes the program using the supplied namespace *
************************************************************************************************************/
subroutine test_cclutExecuteProgramWithMocks_namespace(null)
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the test's documentation does not explain what will be tested or how, I am forced to make something up (guess) based on the name of the test, the documentation quip and the contents of the test.

If I were going to test namespacing, this is what I would do (i.e., here is my guess).

  1. Demonstrate that when "NS" is passed as the third parameter to cclutExecuteProgramWithMocks then
    SUT invokes subroutine NS::SUB defined by the test rather than PUBLIC::SUB defined by SUT when the SUT code invokes SUB (without specifying the namespace). This probably should be accompanied with a test that demonstrates SUT actually invokes PUBLIC::SUB given the same test preparation but not passing the third parameter or passing "", " ", null, "PUBLIC" or "OFF" for the third parameter and probably one that demonstrates SUT's behavior is not affected when SUT invokes PUBLIC::SUB (with the namespace).
  • Just for kicks, I would probably demonstrate something similar for record PUBLIC::rec and variable PUBLIC::var defined by SUT.
  1. Demonstrate that SUT invokes subroutine NS::MOCK_SUB defined by the test instead of PUBLIC::SUB defined by SUT when "NS" is passed for the third parameter to cclutExecuteProgramWithMocks provided that cclutAddMockImplementation("SUB", "mock_sub") has also been called.
  • I suppose the test currently does this, but it will fail if executed in isolation since it relies on some other test to have added the mock implementation. When I read a test, I read it in isolation of anything but setup* and teardown* unless the documentation explains that the test only works if executed after specific other tests have been executed.

On that subject, I would be inclined to have the very initial tests demonstrate that mocks carry forward unless cleared and that they can be cleared. I would then have all subsequent tests clean up mocks at the onset. In reality, I would probably put the tests related to clearing and not clearing in a separate test case(s) and just code setup to perform a clear for all other tests.

Note: I am not that excited about (2) and probably would not have thought of it on my own. If a namespace is being used to divert SUT from PUBLIC::SUB to NS::MOCK_SUB, then there is no need to use a mocked name. It can just as well divert to NS::SUB. The gist of my original comment is that (1) is not tested.

call cclutAddMockData("sample_encounter_alias", "4.0|test alias")

call cclutAddMockImplementation("REPLY", "EXECUTEREPLY")
call cclutAddMockImplementation("InternalSubroutine", "MockSubroutine")
Copy link
Contributor

Choose a reason for hiding this comment

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

sounds like a plan.


The key point here is that the script does not contain a lot of loose code that gets executed every time the script executes. It all gets bundled into the main subroutine which is what allows a unit test to dictate exactly how much of the script's code will be executed.

Here is an illustration of the concept:
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to separate the example into a separate link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I figured it might read better in-line since it wasn't terribly large, but yeah, I can separate all the examples.

set stat = copyrec(mock_other_script_reply, reply, 1)
end go

;;; put the following functions in a test suite (.inc) in src/test/ccl
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the verbiage set up by the founders test case = .inc and test suite = folder full of .inc.

I really tried to change this to have test case = individual test within a .inc and test suite = .inc, but at this point the usage within the code, documentation and common knowledge is too pervasive to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

Below are some examples using the CCL Unit Testing framework's mocking API. These examples assume the script under test is called "the_script" and executes a program called "other_script". They test for scenarios where other_script returns 0 items, more than 5 items, and a failed ("F") status.

```
;;; put the following script definition in a .prg file in src/test/ccl
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's refrain from instructions like this or at least make them alternatives for instructions not involving maven. Many CCL developers have steered away from CCL Unit because the perceived barrier to entry (learning maven) is too high for them. The framework has no dependence on maven or this local file structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll just remove them.


Generally speaking, use "with replace" to mock things that are defined outside the script (CCL subroutines, UARs, other scripts), and use "with curnamespace" to mock things defined within the script.

The CCL Unit Testing framework also supports an abstraction for creating mocks. The purpose is to help make it easier to define mock tables and other mock objects to be used when executing a script. Details on the API can be found at [CCLUTMOCKING.md](../CCLUTMOCKING.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about "provides" rather than "also supports".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.


## Commit/Rollback Guidance

"commit" and "rollback" are keywords within CCL that apply the respective commands to the RDBMS. This can be particularly annoying when dealing with real tables, especially if one is testing insert/update/delete functionality. The table mocking API helps mitigate this by using separate custom tables, but it may still be advantageous to separate any usages of commit/rollback into their own subroutines and test them independently of the other tests.
Copy link
Contributor

Choose a reason for hiding this comment

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

"and test them independently..." is good, but "which can be mocked with no-op subroutines" seems more important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

@cernertrevor
Copy link
Contributor Author

@feckertson It wasn't discussions with a disagreement per se; it was just discussions I had an open question to you on. It's fine though because your latest batch of comments actually does address them. An example was this one:
#7 (comment)

@cernertrevor
Copy link
Contributor Author

Second batch of changes is now available.

@@ -0,0 +1,69 @@
;; this is the test
;;; public::mainForSubroutineB is the workhorse. It will be called instead of public::main because of "with replace"
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's move line 2 into (at the end of) the documentation for testSubroutineB.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure thing.

;assert things here
end ;;;testMyFunctionOtherScriptFail
```
[These examples](./examples/mocking_api.inc) demonstrates the CCL Unit Testing framework's mocking API. They assume the script under test is called "the_script" and executes a program called "other_script". They test for scenarios where other_script returns 0 items, more than 5 items, and a failed ("F") status.
Copy link
Contributor

Choose a reason for hiding this comment

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

number mismatch - these/demonstrates.
Maybe it should be "This example" (one test case) or "These [unit] tests" rather than "These examples".
"they" do not assume the SUT is called "the_script"; "they" test a script named "the_script".... either put the code for the_script in its own file and make "the_script" a link to that file or include the code for the_script in the example file. Other examples use the second approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright. I'll include a base implementation for "the_script" as well as the other changes.

@@ -0,0 +1,70 @@
;;; put the following script definition in a .prg file and compile it
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the introductory example for demonstrating the mocking api.
Shouldn't it foremost include a lightweight demonstration of table and table data mocking which are the key benefits of the api? The introductory paragraph in CCLUTGUIDANCE.md should likewise be updated to mention table access.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, technically the CCLUTMOCKING.md is supposed to be the introduction for mocking. I realize that people can read things in any order and hop around as they please, but this document links to it first, and there's a full example in there using tables.

Would it be more helpful if I said:
"These unit tests demonstrate a basic example of how the CCL Unit Testing framework's mocking API can be used instead of "with replace"."

Or if that's not acceptable, should I just move the example from CCLUTMOCKING here (or create a separate file for it (forgot to do that on the last commit) and link to it from both spots)?

This section was more based on the introduction of what mocking is than the full suite, and I was reusing your example from your blog which was just mocking the script, so my preference would be to clearly state that it's a basic example, or just reuse the CCLUTMOCKING example rather than having a separate example.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it would make more sense if the stated purpose were "how to accomplish a basic 'with replace' while using the mocking framework" opposed to "demonstrate the mocking api".

It would also be good to modify "Details on the API can be found....." to say "Example usage and details...". Otherwise this still feels like the first offering of any example usage of the mocking api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, I'll make those changes.


drop program mock_other_script go
create program mock_other_script
prompt "param 1", "param 2" with param1, param2
Copy link
Contributor

Choose a reason for hiding this comment

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

/**
Mock implementation for other_script which calls validateOtherScriptParams to validate the two input parameters
and increments a counter for the number of calls to other_script.
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

@@ -78,4 +78,7 @@ end

call internalSubroutine(null)

set internalVariable = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose internalVariable and internalRecord->item are used to confirm that the SUT was actually executed.
Is there a situation where that is questionable?
Does it not suffice to just use one thing for this?

Note that the adjective "internal" doesn't really fit here. Maybe "external" or "provided".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose internalVariable and internalRecord->item are used to confirm that the SUT was actually executed.

Actually, I was doing it to comply with the statement from here:
https://github.com/cerner/cclunit-framework/pull/7/files#r250405913

Just for kicks, I would probably demonstrate something similar for record PUBLIC::rec and variable PUBLIC::var defined by SUT.

Although reading it again, it sounds like you wanted the declarations in SUT. Do you want me to just update these tests to move the declarations inside SUT? And, if so, should I leave it as "internal" or do you still want "external"/"provided"?

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct.
That comment intended for the_script to declare a record and a variable to be overridden by the test. I am not sure how to derive value from this capability, but I believe we do support it.

To do this, the _namespace test would declare competing mock::rec and mock::var that would be updated rather than the public::rec and pulic::var declared by SUT. If SUT declares the public:: versions 'with protect', they will fall out of scope after the script returns (i.e., they will fail a validate check) yet the values will be set on the mock:: versions declared by the test.

The 'with persistscript' check needs to be performed on yet some other thing declared within SUT and only needs to be checked by one test after SUT returns I imagine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I'll update the _namespace test and update the _happy test to include a persistscript check.



drop program mock_other_script go
create program mock_other_script
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe document this as
/**
Mock implementation for other_script which sets reply equal to a copy of mock_other_script_reply.
*/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

subroutine test_cclutExecuteProgramWithMocks_happy(null)
declare public_subroutine = i4 with protect, noconstant(0)
declare mock_subroutine = i4 with protect, noconstant(0)
declare test_subroutine = i4 with protect, noconstant(0)
declare public::internalVariable = i4 with protect, noconstant(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any examples where the SUT declares the reply with persistscript (which is another way this "internalRecord" business could be handled)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have any specifically using "with persistscript", no. I can do that if you don't want me to move the declarations into the script per this question:
#7 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it (persistscript) would be a good thing to cover with the tests. I think it would be convenient to use this specific "reply" structure for that purpose.

/* test_cclutAddMockImplementation_not_cleared **************************************************************
* Scenario: Demonstrates that the mocks from the previous test still exist since they have not been cleared*
************************************************************************************************************/
subroutine test_cclutAddMockImplementation_not_cleared(null)
Copy link
Contributor

Choose a reason for hiding this comment

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

What guarantees that _happy executes before _not_cleared?
Test order is not a concern if the check is done in teardownOnce instead. If fact setupOnce and different tests could all add different mocks and teardownOnce could check that they accumulated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will make the changes.

@@ -0,0 +1,295 @@

# CCL Unit Mocking
Copy link
Contributor

Choose a reason for hiding this comment

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

I just looked at the pretty view of this file for the first time. It seems like it would be better to have a brief high-level description of what CCL Unit Mocking is all about in the location of the warning and put the warning after that. It might also be good to have a TOC just below the description making it easy to jump directly to the warning, spec, implementation notes and example.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

@cernertrevor
Copy link
Contributor Author

Third batch of changes is now available.

…as an enhancement on Github after this PR is merged.
@cernertrevor
Copy link
Contributor Author

@feckertson
Do you have any more thoughts or recommendations for improvement to the API?

@cernertrevor
Copy link
Contributor Author

Actually, I just noticed there was a conflict (likely from the merging of the other issue). Let me resolve that first.


## CCL Mocks

In general, there are two ways to mock objects in CCL unit tests. Generally speaking, use "with replace" to mock things that are defined outside the script or called directly by the script (CCL subroutines, UARs, other scripts), and use "with curnamespace" to mock subroutines executed by the subroutine being tested. When using "with curnamespace", add the PUBLIC namespace to the real thing and use an alternate namespace to define an override. Execute the script using 'with curnamespace = "\<alternate namespace\>"'. In practice, it is convenient to use the name of the test case for the alternate namespace.
Copy link
Contributor

Choose a reason for hiding this comment

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

use the name of the test for the alternate namespace (not the name of the test case (inc file))


In general, there are two ways to mock objects in CCL unit tests. Generally speaking, use "with replace" to mock things that are defined outside the script or called directly by the script (CCL subroutines, UARs, other scripts), and use "with curnamespace" to mock subroutines executed by the subroutine being tested. When using "with curnamespace", add the PUBLIC namespace to the real thing and use an alternate namespace to define an override. Execute the script using 'with curnamespace = "\<alternate namespace\>"'. In practice, it is convenient to use the name of the test case for the alternate namespace.

The CCL Unit Testing framework provides an abstraction for creating mocks. The purpose is to help make it easier to define mock tables and other mock objects to be used when executing a script. Details on the API can be found at [CCLUTMOCKING.md](../CCLUTMOCKING.md)
Copy link
Contributor

Choose a reason for hiding this comment

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

"help make it easier" could simply be "make it easier"


The CCL Unit Testing framework provides an abstraction for creating mocks. The purpose is to help make it easier to define mock tables and other mock objects to be used when executing a script. Details on the API can be found at [CCLUTMOCKING.md](../CCLUTMOCKING.md)

[These unit tests](./examples/mocking_api.inc) demonstrate how to accomplish a basic "with replace" while using the CCL Unit Testing framework's mocking API. They test a script named "the_script" and executes a program called "other_script". They test for scenarios where other_script returns 0 items, more than 5 items, and a failed ("F") status.
Copy link
Contributor

Choose a reason for hiding this comment

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

"and executes" should be "which executes"


[These unit tests](./examples/mocking_api.inc) demonstrate how to accomplish a basic "with replace" while using the CCL Unit Testing framework's mocking API. They test a script named "the_script" and executes a program called "other_script". They test for scenarios where other_script returns 0 items, more than 5 items, and a failed ("F") status.

There are other variations on this. For example, you could put asserts within mock_other_script itself. Additionally, other_script might generate its own reply structure, so you would want to do the same in mock_other_script.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unless one follows the links as they are presented, this is the first mention of "mock_other_script". To make this less startling, the previous paragraph could say 'They leverage a script named "mock_other_script" to mock the behavior of "other_script" and test "the_script" in scenarios where "other_script" returns 0 items, more than 5 items and a failed ("F") status.

# CCL Unit Mocking
The CCL Unit Test framework's mocking API is available for consumers to mock certain objects to better isolate unit tests from outside variability and provide more control over the scenarios under which unit tests are run. The API provides ways to mock tables, variables, subroutines, scripts, etc.

**\*CAUTION\*** **The CCL Unit Mocking framework should only be used in non-production environments. Table mocking creates new tables against an Oracle instance for the lifetime of the test. Because the DDL is generated in a dynamic way, it is possible through inappropriate use of the framework to affect the actual table. Please only use the documented API.**
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be "an actual table" rather than "the actual table" ?
There is also a chance that the mock tables will not be cleaned up (because CCL crashes mid-test). Should that statement be woven in as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this be "an actual table" rather than "the actual table" ?

Yeah, that makes sense.

There is also a chance that the mock tables will not be cleaned up (because CCL crashes mid-test). Should that statement be woven in as well?

Sure... though I might throw in a note that this is extremely rare (I've never, to my knowledge, had interactive CCL crash on me, only script servers) and that this is different than just simple errors (Some clients I've met weren't aware that CCL keeps processing after errors).

Copy link
Contributor

Choose a reason for hiding this comment

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

Hit CTRL-C twice in the middle of a maven build. The current backend session will be terminated and no cleanup will occur.

I have lost track of the example, but at one time I knew of an innocent construct that would cause CCL to abort. I imagine there still is one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I actually just pushed up my next batch of changes which has my proposed wording for this. Feel free to look it over and let me know if you want me to change any of it.

&nbsp;&nbsp;&nbsp;&nbsp;A pipe-delimited string of data to be inserted into the mock table.

Example:
call cclutDefineMockTable("person", "person_id|name_last|name_first|birth_dt_tm", "f8|vc|vc|dq8")
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about setting off all examples as code fragments using ` or ``` or ```javascript ?
Observe that backslashes should not be escaped inside of code fragments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.


**cclutClearMockData(tableName = vc)**

Clears all data from the mock table. This is functionally similar to a truncate. tableName is required. The table
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see this change.


**cclutRemoveAllMocks**

Removes all mock implementations and mock tables that have been added through the cclutAddMockImplementation() and cclutCreateMockTable() APIs. This should be called at the completion of a test suite to clean up all mocks.
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be "test case" rather than "test suite".

It seems like the warning statement ("cclutRemoveAllMocks should be called at the completion....") is lost in the middle of this file. Perhaps it should be made at the beginning. ???

One option is to place the cclutRemoveAllMocks documentation right after the documentation for cclutExecuteProgramWithMocks and simply say "through the mocking APIs" or "through the mocking APIs described below" rather than citing specific functions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.


declare mock_table_person = vc with protect, noconstant("")

; Defining a mock person table. The return value is the name of the mocked table.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this comment about the return value from cclutDefineMockTable should just be a final statement in the documentation for cclutDefineMockTable.

Returns a string that is the name of the mock table. This....

Note: it should be "mock table" rather than "mocked table"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have an @returns tag in the cclutDefineMockTable documentation. Are you saying you want this included in the "description" part?

call cclutAddMockData("person", "2.0|Adams|John|02-FEB-1971 11:11") ;Will add John Adams
call cclutAddMockData("person", "3.0|Jefferson|\null|03-MAR-1972 22:22") ;Will add Jefferson (no first name)
call cclutAddMockData("person", "4.0|Madison||04-APR-1973 10:33") ;Will add Madison (empty string for first name)

Copy link
Contributor

Choose a reason for hiding this comment

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

Given the lengthy comment about the return value from cclutDefineMockTable, one would expect this test to actually make use of the mock_table_person value, but it doesn't.

Although I advocate moving that discussion to the cclutDefineMockTable documentation, it seems reasonable to provide a demonstration somewhere. It is somewhat contrived, but selecting the count of records from mock_table_person at this point would suffice. It could even be set off with something like ";;; a contrived example to demonstrate a potential use for the return value from cclutDefineMockTable".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.

feckertson and others added 4 commits February 12, 2019 15:45
…into ccl-mock

# Conflicts:
#	cclunit-framework-source/src/test/ccl/ut_cclut_execute_test_case.inc
#	cclunit-framework-source/src/test/ccl/ut_cclut_execute_test_case_file.inc
# Conflicts:
#	cclunit-framework-source/src/test/ccl/ut_cclut_execute_test_case.inc
#	cclunit-framework-source/src/test/ccl/ut_cclut_execute_test_case_file.inc
…into ccl-mock

# Conflicts:
#	cclunit-framework-source/src/test/ccl/ut_cclut_execute_test_case.inc
#	cclunit-framework-source/src/test/ccl/ut_cclut_execute_test_case_file.inc
@@ -28,11 +30,25 @@ cclutExecuteProgramWithMocks.
&nbsp;&nbsp;&nbsp;&nbsp;The namespace under which to execute the program.

Example:
call cclutExecuteProgramWithMocks("ccl_my_program", "\^MINE^, 1.0, ^string parameter^", "MYNAMESPACE")
```
Copy link
Contributor

Choose a reason for hiding this comment

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

You didn't like the javascript version? It is kind of colorful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh, I can add it to all of them if you'd prefer. I figured since it wasn't actually JavaScript, some of the keywords might be incorrectly colored/not colored, so it might be better just to have nothing. Some things (like strings) would benefit though.

Copy link
Contributor

Choose a reason for hiding this comment

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

It stands out a lot better.
A global search/replace for Example:\s*[\n\r]*``` should catch most of them.
Do you have a markdown viewer plugin for your browser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do, though I typically use the one in my IDE. I'll update them to javascript.

**\*CAUTION\*** **The CCL Unit Mocking framework should only be used in non-production environments. Table mocking creates new tables against an Oracle instance for the lifetime of the test. Because the DDL is generated in a dynamic way, it is possible through inappropriate use of the framework to affect the actual table. Please only use the documented API.**
**\*CAUTION\*** **The CCL Unit Mocking framework should only be used in non-production environments. Table mocking creates new tables against an Oracle instance for the lifetime of the test. Because the DDL is generated in a dynamic way, it is possible through inappropriate use of the framework to affect an actual table. Please only use the documented API.**

In the rare event that CCL crashes midway through a test or another abnormal abort occurs (e.g. as the result of an infinite loop in a test), it may be necessary to clean up any tables that the framework could not. All tables created by the CCL Unit Test framework will be prepended with "CUST_CCLUT_".
Copy link
Contributor

Choose a reason for hiding this comment

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

I am fine with doing it this way, by the way.

@feckertson
Copy link
Contributor

I am going to submit a DevCon talk proposal for CCL Unit. Do you have any interest in covering the mocking api? If not, I'll be sure to give you credit when I cover it. No big deal one way or the other.

@cernertrevor
Copy link
Contributor Author

Yes, I think that's a great idea, and I'd love to do the mocking portion. Since I work remote, my team needs to work out some of the logistics/budget, but my manager says he'll push hard for that, so let's assume yes. If the proposal gets approved, we can go from there.

@feckertson
Copy link
Contributor

Okay. I'll list you as a co-presenter then.

Copy link
Contributor

@feckertson feckertson left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks again for doing all this.

@feckertson feckertson merged commit 6d98b87 into oracle-samples:master Feb 15, 2019
@cernertrevor
Copy link
Contributor Author

Not a problem. Thank you for all your recommendations and insight. I'll create a release PR soon.

@feckertson
Copy link
Contributor

I can take care of the release. I want to drop the cclunit-framework-tests and reactor projects and rework the front door documentation a little.

@cernertrevor
Copy link
Contributor Author

Oh ok, sure thing. Thanks.

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.

None yet

2 participants