Improve unit testing and stability of inactive users command

core/Command/User/Inactive.php

@@ -55,15 +55,17 @@ protected function configure() {
 
 	protected function execute(InputInterface $input, OutputInterface $output) {
 		$days = $input->getArgument('days');
-		if($days < 1) {
-			throw new InvalidArgumentException('Days must be above zero');
+		if(!is_int($days) || $days < 1) {
+			throw new InvalidArgumentException('Days must be integer and above zero');
 		}
 
+		$now = time();
+
 		$inactive = [];
-		$this->userManager->callForSeenUsers(function(IUser $user) use (&$inactive, $days) {
+		$this->userManager->callForSeenUsers(function(IUser $user) use (&$inactive, $days, $now) {
 			$lastLogin = $user->getLastLogin();
 			// Difference between now and last login, into days, and rounded down
-			$daysSinceLastLogin = floor((time() - $lastLogin) / (60*60*24));
+			$daysSinceLastLogin = floor(($now - $lastLogin) / (60*60*24));
 			if($daysSinceLastLogin >= $days) {
 				$inactive[] = [
 					'uid' => $user->getUID(),

tests/Core/Command/User/InactiveTest.php

@@ -73,12 +73,41 @@ public function testWithInvalidDays($days) {
 
 		self::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
 	}
+
+	protected function dummyUser($lastLogin) {
+		$user = $this->createMock(IUser::class);
+		$user->expects($this->once())
+			->method('getLastLogin')
+			->willReturn($lastLogin);
+
+		return $user;
+	}
+
+	public function validDays() {
+		return [
+			'no users' => [[], 10, 0],
+			'1 recent user, excluded' => [[$this->dummyUser(time()-2*24*60*60)], 10, 0],
+			'1 recent user, included' => [[$this->dummyUser(time()-2*24*60*60)], 1, 1],
+			'2 users 1 includd' => [
+				[
+					$this->dummyUser(time()-5*24*60*60),
+					$this->dummyUser(time()-10*24*60*60)
+				], 7, 1],
+		];
+	}
 
-	public function testWithValidDays() {
+	/**
+	 * @dataProvider validDays
+	 *
+	 * @param $users
+	 * @param $days
+	 * @param $expectedCount
+	 */
+	public function testWithValidDays($users, $days, $expectedCount) {
 		$this->consoleInput->expects($this->once())
 			->method('getArgument')
 			->with('days')
-			->willReturn(1);
+			->willReturn($days);
 
 		// Work with json output
 		$this->consoleInput->expects($this->once())
@@ -87,11 +116,18 @@ public function testWithValidDays() {
 			->willReturn(Base::OUTPUT_FORMAT_JSON);
 
 		$this->userManager->expects($this->once())
-			->method('callForSeenUsers');
+			->method('callForSeenUsers')
+			->willReturnCallback(function($callback) use ($users, $expectedCount) {
+				foreach($users as $user) {
+					$callback($user);
+				}
+			});
 
 		$this->consoleOutput->expects($this->once())
 			->method('writeLn')
-			->with($this->stringContains('['));
+			->with($this->callback(function($output) use ($expectedCount) {
+				return self::isJson($output) && count(json_decode($output)) === $expectedCount;
+			}));
 
 		self::invokePrivate($this->command, 'execute', [$this->consoleInput, $this->consoleOutput]);
 	}