Refactoring tests with skip() and skipAll(), help wanted

Zackster · July 30, 2022, 1:25pm

I have proposed an enhancement for Testbox, to add skip and skipAll() functionality, so we can cleanup some of the test handling in Lucee, when services aren’t available

https://ortussolutions.atlassian.net/browse/TESTBOX-345

Once this is available, I’d love some help from the community to go thru and refactor/cleanup all our tests

An example, see all the isNotSupported() cruft / boilerplate in this testcase?

github.com

lucee/Lucee/blob/6.0/test/datasource/MongoDB.cfc#L57


      
          		];
          
          
		coll.drop();
          		coll.insert(docs);
          		return coll;
          	}
          
          
	// public function setUp(){}
          
          
	public function beforeTests() skip="isNotSupported" {
          		if(isNotSupported()) return;
          		var uri = "mongodb://#variables.mongoDB.server#:#variables.mongoDB.port#";
          		if (!isempty(variables.mongoDB.username) && !isEmpty(variables.mongoDB.password))
          			uri = "mongodb://#variables.mongoDB.username#:#variables.mongoDB.password#@#variables.mongoDB.server#:#variables.mongoDB.port#";
          
          
		//systemOutput("MongoDB URI: " & uri, true, true);
          
          
		// test host/port via http, should return: "It looks like you are trying to access MongoDB over HTTP on the native driver port."
          		/*
          		try {
          			http method="GET" url="http://#variables.mongoDB.server#:#variables.mongoDB.port#/#variables.mongoDB.db#" result="local.httpRes";

it could all be stripped out with

public function beforeTests() {
    if ( len( server.getDatasource( "mongoDB" ) ) eq 0 )
        skipAll();
}

much cleaner eh?

Gavin_Baumanis · August 1, 2022, 12:25am

Hi @Zackster,
I am now watching the ticket - once it is completed I can help with some work.

Zackster · August 1, 2022, 7:50am

Having thought about this over the weekend, in the spirit of removing boilerplate, we could add services as component metadata

component extends="org.lucee.cfml.test.LuceeTestCase" labels="mssql" services="mssql" {
}

basically following the same logic we use for labels

github.com

lucee/Lucee/blob/6.0/test/_testRunner.cfc#L105


      
          	} // onBundleStart = function
          	,onBundleEnd = function( cfc, testResults ){
          		var bundle = arrayLast( testResults.getBundleStats() );
          		var oh = ( getTickCount() - request._tick )-bundle.totalDuration;
          		request._tick = getTickCount();
          		ArrayAppend( request.overhead, oh );
          		try {
          			SystemOut.setOut( out );
          			//SystemOut.setErr(err);
          			if ( bundle.totalPass eq 0 && ( bundle.totalFail + bundle.totalError ) eq 0 ){
          				systemOutput( TAB & " (skipped)", true );
          			} else {
          				var skippedSummary = (bundle.totalSkipped gt 0) ? ", #bundle.totalSkipped# skipped" : "";
          				systemOutput( TAB & " (#bundle.totalPass# tests passed in #NumberFormat(bundle.totalDuration)# ms#skippedSummary#)", true );
          			}
          			//mem("non_heap");
          			//mem("heap");
          		// we have an error
          		if ( ( bundle.totalFail + bundle.totalError ) > 0 ) {
          
          
			systemOutput( "ERRORED" & NL & "	Suites/Specs: #bundle.totalSuites#/#bundle.totalSpecs#

Gavin_Baumanis · August 2, 2022, 12:45am

Hi @Zackster

Do we have a “coding style guide”?
Eg.

Use 4 space TABS
have a space BEFORE and AFTER parenthesis
All new code MUST have a test-case to cover changes.
All existing tests MUST include required updates as a result of your changes.

And more to the point for THIS thread;
When writing a test;
We use the following attribuites; “tags” / “services” / “etc”…
The “tag” attributes is for… which does “this” during the CI process…

I did look through the non-source code documents in the root of the Lucee repo - but couldn’t find anything.

Gavin.

Zackster · August 2, 2022, 6:05am

There are no current style guidelines for unit tests, what you describe is my normal coding style for tests I am too old to even touch this topic, even with a barge pole!

Some of our tests are a bit ugly, but I’m of the school, merge when reasonable, as once it’s merged, others can then (theoretically) dive in and improve / cleanup tests.

As for tests being required, it’s simply standard practice these days and it’s clearly mentioned on Lucee/CONTRIBUTING.md at 6.0 · lucee/Lucee · GitHub

We generally won’t merge any functionality changes which doesn’t have test coverage, however, not everything can be unit tested due to various reasons (JEE sessions for example, aren’t available when running tests) and underlying bug fixes / improvements to the core engine.

We could do with some documentation on tests in Lucee, it’s all copy by example at the moment