Skip to content

Commit

Permalink
Check for a yui/src directory when running the Grunt YUI task
Browse files Browse the repository at this point in the history
This commit solves an issue in some repositories where the following
message is shown:

    Unable to find local grunt

This message is a little confusing because the Grunt uses both a global,
and a local copy of the Grunt executable. The global variant comes from
the `npm install grunt-cli` Installation task, whilst the local variant
is a dependency of Moodle.

The message actually occurs when the global executable is unable to find
the local variant from the current working directory.

To give an example, given the following directory:

    /home/travis/build/moodle/local/chatlogs/yui/src

Grunt will check each of the parent directories until it finds a
`Gruntfile.js` and/or relevant `node_modules/.bin/grunt` executable, i.e.:

    /home/travis/build/moodle/local/chatlogs/yui/src
    /home/travis/build/moodle/local/chatlogs/yui
    /home/travis/build/moodle/local/chatlogs
    /home/travis/build/moodle/local
    /home/travis/build/moodle
    /home/travis/build
    /home/travis
    /home
    /

If none is found then the "Unable to find local grunt" message is then
shown.

In the issue where this fault was detected, the plugin does have a `yui`
directory, but it uses the legacy module structure where the YUI module
is placed directly into the directory, and not within a Shifted module
structure. That is to say that the following is the actual directory
structure of the plugin:

```
└── yui
    ├── jabberaliases
    │   └── jabberaliases.js
    └── keyboard
        └── keyboard.js
```

This is a perfectly valid YUI structure, but Grunt is unaware of it, and
its use has not been recommended for many years. There is no `src`
directory present, and each module is just in it's own directory outside
of the src structure.

The Grunt Command was configured to perform the following when
processing a `yui` task:

```
case 'yui':
    $yuiDir = $this->plugin->directory.'/yui';
    if (!is_dir($yuiDir)) {
        return null;
    }

    return new GruntTaskModel($task, $yuiDir.'/src', 'yui/build');
```

Essentially it assumes that if the `yui` directory exists then the task
should pass a workingDirectory for `yui/src`.

In this case there is no yui/src directory, but the full path is passed
as a working directory to the `ProcessBuilder`.  The ProcessBuilder does
not care that the directory does not exist (presumably because it may
not exist during build, but may do prior to execution), but the result
is that `grunt` is called with a non-existent working directory.

It is not possible to traverse anywhere where the working directory does
not exist, and so the local Grunt cannot be found.

The solution here is to always look for `yui/src` within the plugin
directory.

Grunt is not aware of the older-style directories and does not perform
any checks on those so there is no loss of functionality.

Fixes open-lms-open-source#45
  • Loading branch information
andrewnicols authored and stronk7 committed Oct 20, 2020
1 parent eb3c3a0 commit f9a761b
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 3 deletions.
5 changes: 4 additions & 1 deletion docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ This project adheres to [Semantic Versioning](http://semver.org/).
The format of this change log follows the advice given at [Keep a CHANGELOG](http://keepachangelog.com).

## [Unreleased]
No unreleased changes.
### Fixed
- `moodle-plugin-ci grunt` now only runs against the `yui/src` directory when configuring the YUI task.
This resolves an issue where an "Unable to find local grunt" message was reported when code was structured in a legacy
format. See #46 for more details.

## [3.0.3] - 2020-10-16
### Changed
Expand Down
4 changes: 2 additions & 2 deletions src/Command/GruntCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -184,12 +184,12 @@ public function toGruntTask($task)
return new GruntTaskModel($task, $amdDir, 'amd/build');
case 'shifter':
case 'yui':
$yuiDir = $this->plugin->directory.'/yui';
$yuiDir = $this->plugin->directory.'/yui/src';
if (!is_dir($yuiDir)) {
return null;
}

return new GruntTaskModel($task, $yuiDir.'/src', 'yui/build');
return new GruntTaskModel($task, $yuiDir, 'yui/build');
case 'gherkinlint':
if ($this->moodle->getBranch() < 33 || !$this->plugin->hasBehatFeatures()) {
return null;
Expand Down
10 changes: 10 additions & 0 deletions tests/Command/GruntCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ public function testToGruntTaskWithYUI()
$this->assertNull($command->toGruntTask('shifter'));
}

public function testToGruntTaskWithLegacyYUI()
{
$command = $this->newCommand();
$this->fs->remove($this->pluginDir.'/yui/src');
$this->fs->touch($this->pluginDir.'/yui/examplejs');

$this->assertNull($command->toGruntTask('yui'));
$this->assertNull($command->toGruntTask('shifter'));
}

public function testToGruntTaskWithGherkin()
{
$command = $this->newCommand();
Expand Down

0 comments on commit f9a761b

Please sign in to comment.