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.

Sign up for GitHub

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

Jump to bottom

28-integration-in-ci #29

Closed
wants to merge 2 commits into from
Closed

28-integration-in-ci #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
37b3540
#28 add ci flag
khalyomede Jan 24, 2022
febac85
#28 add test case
khalyomede Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Commands/LocalizeCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class LocalizeCommand extends Command
*
* @var string
*/
protected $signature = 'localize {lang?}';
protected $signature = 'localize {lang?} {--c|ci : Returns a non zero code when some translations have been added.}';

/**
* The console command description.
Expand Down Expand Up @@ -65,7 +65,7 @@ public function handle(Localizator $localizator, Parser $parser): int
"\nTranslatable strings have been generated for locale(s): ".implode(', ', $locales)
);

return 0;
return $this->option("ci") && $parser->foundSomeKeys() ? 1 : 0;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/Services/Parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ public function parseKeys(): void
});
}

public function foundSomeKeys(): bool
{
return ($this->defaultKeys->count() + $this->jsonKeys->count()) > 0;
}

/**
* @param $key
* @return bool
Expand Down
40 changes: 40 additions & 0 deletions tests/LocalizatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,4 +165,44 @@ public function testLocalizeCommandWhereKeysAreEscapedWithSlashes(): void
// Cleanup.
self::flushDirectories('lang', 'views');
}

public function testLocalizeCommandReturnsNonZeroCodeWhenCiFlagIsOnAndSomeMissingTranslationsHaveBeenAdded(): void
{
$this->createTestView("{{ __('Hi mom') }}");

// Run localize command.
$this->artisan('localize', ['--ci' => true])
->assertExitCode(1);

// Cleanup.
self::flushDirectories('lang', 'views');
}

public function testLocalizeCommandReturnsZeroCodeWhenCiFlagIsOnAndNoMissingTranslationsHaveBeenFound(): void
{
$this->createTestView("");

// Run localize command.
$this->artisan('localize', ['--ci' => true])
->assertExitCode(0);

// Cleanup.
self::flushDirectories('lang', 'views');
}

public function testLocalizeCommandReturnsNonZeroCodeThenZeroCodeIfFileDidNotChangedBetweenTwoRun(): void
Copy link
Author

Choose a reason for hiding this comment

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

This one will not pass, because I feel I will need something new that seems to not exist yet. Let me explain:

For this test to pass, the parser should detect, among the keys it has collected, which ones are not yet in all translated files. This seems like this piece of logic mixes both the Parser and the Writers, since the Writers are aware of which translations files contains which keys, when the Parser only knows which keys are in the "app" files.

My first suggestion would be to have a method Parser::foundSomeNewKeysComparedTo(Collection $keys): bool.

What do you think about it @amiranagram?

Copy link
Owner

Choose a reason for hiding this comment

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

Good job overall!

As for this you can use Amirami\Localizator\Services\Localizator class to merge the keys together and figure out if there are any. Look at the collect method there, it does everything. You can make it public.

So like:

$localizator->collect($parser->getKeys($locale, $type), $type, $locale);

Obviously need to loop over all types and locales.

{
$this->createTestView("{{ __('Hi mom') }}");

// Run localize command.
$this->artisan('localize', ['--ci' => true])
->assertExitCode(1);

// Run localize command.
$this->artisan('localize', ['--ci' => true])
->assertExitCode(0);

// Cleanup.
self::flushDirectories('lang', 'views');
}
}