Now I go ahead and start using this service. It quickly becomes obvious that some of the constants are also used in a map to display a human readable status to the user:
xxx scenarios (xx passed, 112 failed)
xxxx steps (xxxx passed, 49 failed, xxx skipped)
xxmxx.xxs (90.59Mb)
What we want then, is to remove this one constant. Let’s just try that and see what breaks? Here’s the output from phpunit:
/**
* Gets the state key of a node.
*/
function _cronner_get_state_key(NodeInterface $node) {
return CRONNER_STATE_KEY_PREFIX . $node->id();
}
To celebrate this tiny step towards getting rid of cronner.module I tried to search for cronner on giphy. No hits, at this point. Oh well, here is an animated gif that supposedly illustrates “cronn”
But looking at these old toys, it also becomes obvious what we use these constants for. Node state could mean all kinds of things, right? This is used to store and update the job status of the projects. Like if they are currently queued, if they are currently running and so on.
- cronner_set_state($node, CRONNER_PROJECT_PROCESSED);
+ /** @var Drupalviolinist_projectsProjectRunStatus $run_status_service */
+ $run_status_service = Drupal::service('violinist_projects.run_status');
+ $run_status_service->setRunStatusForProject($node, ProjectRunStatusValue::STATUS_PROCESSED);
The first lines of the module file is this:
xxx Function cronner_set_state not found.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
xxx Function _cronner_get_state_key not found.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
xxx Function cronner_get_human_status not found.
💡 Learn more at https://phpstan.org/user-guide/discovering-symbols
So it turns out that while removing this, this actually seems like a good opportunity to clean up some of that code and make it a bit more modern. What I usually do when cleaning up custom code like this is to put as much as possible on drupal.org as open source code. This is the case now as well. So I open an issue to create a project run status service, and start to refactor the custom code and logic surrounding the removals of constants and functions in cronner.module. It can be found here: https://www.drupal.org/project/violinist_projects/issues/3453459.
<?php
/**
* @file
* Cronner module.
*
* Bad name, but what are you going to do, right?
*/
ReplaceTokensTest::testClaimJobAndTokenReplaced
Error: Call to undefined function cronner_set_state()
The first few lines of actual code are constant definitions. Here’s the first one:
define('CRONNER_STATE_KEY_PREFIX', 'cronner_state.node.');
Function _cronner_get_state_key not found.
…
[ERROR] Found 7 errors
Background: This blog post is part 2 of musings around legacy code on violinist.io. Violinist.io is an automated dependency updater for PHP/Composer. It’s a SaaS built on Drupal 8, now running on Drupal 10, in its eigth year. In this second post I am looking at one way to approach legacy code, and how to use static analysis and test driven development to safely refactor and remove legacy code.
Line web/modules/custom/cronner/cronner.module
------ ---------------------------------------------------------------------
xxx Constant CRONNER_STATE_KEY_PREFIX not found.
/**
* Gets a human readable version of a status constant.
*
* @param string $status
* A status constant.
*
* @return string
* Something more sensible.
*/
function cronner_get_human_status($status) {
$map = [
CRONNER_PROJECT_UNKNOWN => t('Unknown'),
CRONNER_PROJECT_ERRORED => t('Errored'),
CRONNER_PROJECT_PROCESSED => t('Processed'),
CRONNER_PROJECT_RUNNING => t('Running'),
CRONNER_PROJECT_NEW => t('New'),
CRONNER_PROJECT_QUEUED => t('Queued'),
];
return !empty($map[$status]) ? $map[$status] : $status;
}
define('CRONNER_PROJECT_NEW', 'new');
define('CRONNER_PROJECT_QUEUED', 'queued');
define('CRONNER_PROJECT_RUNNING', 'running');
define('CRONNER_PROJECT_PROCESSED', 'processed');
define('CRONNER_PROJECT_ERRORED', 'errored');
define('CRONNER_PROJECT_UNKNOWN', 'unknown');
Some months ago I wrote about the combined feeling of shame and respect that surrounds legacy code. In an attempt to get rid of the legacy code in what is called cronner.module, I will start from the top of the .module file, and work my way towards removing it completely. From time to time, that can also result in a good blog post, I thought. Here’s a blog post about that.
There were 19 errors:
…
Error: Undefined constant "CRONNER_STATE_KEY_PREFIX”
The unit tests are failing. 19 of them. Good start, we have decent unit test coverage. Let’s see if our integration tests fail?
