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

Conversation

@cernertrevor
Copy link
Contributor

commented Dec 13, 2018

@cernertrevor

This comment has been minimized.

Copy link
Contributor Author

commented Dec 13, 2018

@feckertson
Copy link
Contributor

left a comment

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 3, 2019

Author Contributor

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")

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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?

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 3, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 3, 2019

Author Contributor

Sure thing.


**cclutClearMockData(tableName = vc)**

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

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

"...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")

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 3, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

I don't see this change.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 13, 2019

Author Contributor

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

* Scenario: Executes the program while replacing a subroutine with a mocked version *
************************************************************************************************************/
subroutine test_cclutExecuteProgramWithMocks_mock_subroutine(null)
call echo("~~~***~~~***test_cclutExecuteProgramWithMocks_mock_subroutine***~~~***~~~")

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 24, 2019

Author Contributor

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?

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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).

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 30, 2019

Author Contributor

Alright, sure.

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

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

the asserts before here can be safely omitted.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 24, 2019

Author Contributor

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 *

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 24, 2019

Author Contributor

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}">

This comment has been minimized.

Copy link
@feckertson

feckertson Dec 15, 2018

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 24, 2019

Author Contributor

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.

@feckertson
Copy link
Contributor

left a comment

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

This comment has been minimized.

Copy link
Contributor Author

commented Jan 29, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2019

@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))

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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 *****************************************************

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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 *
************************************************************************************************************/

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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")

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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:

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 30, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 31, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 31, 2019

Author Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 31, 2019

Author Contributor

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Jan 29, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Jan 31, 2019

Author Contributor

Sure thing.

@cernertrevor

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2019

@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

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2019

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"

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

/**
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.
*/

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

Will change.

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

call internalSubroutine(null)

set internalVariable = 1

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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".

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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"?

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 4, 2019

Author Contributor

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

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 1, 2019

Author Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 4, 2019

Author Contributor

Will make the changes.

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

# CCL Unit Mocking

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 1, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 4, 2019

Author Contributor

Will do.

@cernertrevor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 5, 2019

Third batch of changes is now available.

Removal of cclutAddConstraint API for initial version. Will be added …
…as an enhancement on Github after this PR is merged.
@cernertrevor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

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

@cernertrevor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 12, 2019

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

"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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

"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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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.**

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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?

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 12, 2019

Author Contributor

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).

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 14, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 14, 2019

Author Contributor

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")

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 13, 2019

Author Contributor

Sure.


**cclutClearMockData(tableName = vc)**

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

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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.

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 13, 2019

Author Contributor

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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"

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 13, 2019

Author Contributor

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)

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 12, 2019

Contributor

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".

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 13, 2019

Author Contributor

Will do.

feckertson and others added 4 commits Feb 12, 2019
Merge branch 'master' of https://github.com/cerner/cclunit-framework
…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
Merge remote-tracking branch 'origin/ccl-mock' 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
Merge branch 'master' of https://github.com/cerner/cclunit-framework
…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")
```

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 14, 2019

Contributor

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

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 14, 2019

Author Contributor

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.

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 14, 2019

Contributor

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?

This comment has been minimized.

Copy link
@cernertrevor

cernertrevor Feb 14, 2019

Author Contributor

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_".

This comment has been minimized.

Copy link
@feckertson

feckertson Feb 14, 2019

Contributor

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

@feckertson

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Feb 14, 2019

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

This comment has been minimized.

Copy link
Contributor

commented Feb 14, 2019

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

@feckertson
Copy link
Contributor

left a comment

Looks great. Thanks again for doing all this.

@feckertson feckertson merged commit 6d98b87 into cerner:master Feb 15, 2019

@cernertrevor

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2019

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

@feckertson

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

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

This comment has been minimized.

Copy link
Contributor Author

commented Feb 15, 2019

Oh ok, sure thing. Thanks.

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