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.

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

WIP: github actions phpunit #3172

Closed
wants to merge 13 commits into from
Closed

WIP: github actions phpunit #3172

wants to merge 13 commits into from

Conversation

bloep
Copy link
Member

@bloep bloep commented Jan 18, 2020

closes #3169

@@ -112,3 +112,26 @@ jobs:
extensions: intl
coverage: none # disable xdebug, pcov
- run: composer global require friendsofphp/php-cs-fixer:2.14.* && ~/.composer/vendor/bin/php-cs-fixer fix --diff --dry-run

phpunit:
Copy link
Member

@staabm staabm Jan 19, 2020

Choose a reason for hiding this comment

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

Den neuen build brauchen wir, weil wir vorher phpunit nur in travis hatten?

Falls ja, kann jetzt ein travis build weg?
(Langfristig hätte ich am liebsten alle travis builds weg, und stattdessen github-actions haben - weil schneller)

Copy link
Member Author

Choose a reason for hiding this comment

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

absolut, phpunit hatten wir bisher nur im travis.
wenn das hier funktioniert, würde ich den gleichen travis check entfernen.


// use different result printer with github actions checks integration
if (getenv('GITHUB_ACTIONS')) {
echo "Running in Github Actions\n";
Copy link
Member

@staabm staabm Jan 19, 2020

Choose a reason for hiding this comment

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

Ich vermute, diese ausgabe führt zu headers already sent errors

Suggested change
echo "Running in Github Actions\n";

Copy link
Member

Choose a reason for hiding this comment

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

Hmm nee

Copy link
Member Author

Choose a reason for hiding this comment

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

tatsächlich habe ich es auch lokal, sobald ich den anderen Printer nehme, dass die Warnings/errors kommen.
Kann es sein, dass die beim anderen result printer irgendwie unterdrückt werden?

Copy link
Member

Choose a reason for hiding this comment

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

ich vermute dass es failed, weil die meldungen ein newline enthalten und daher direkt geflushed werden

Copy link
Member

Choose a reason for hiding this comment

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

bin heut nicht gut im raten.. :-)


$message = explode(PHP_EOL, $e->getMessage())[0];

$this->write("::{$this->getCurrentType()} file={$this->relativePath($path)},line={$line}::{$message}\n");
Copy link
Member

Choose a reason for hiding this comment

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

Hmm ne, vermutlich sind die ausgaben hier der grund...hmm

Copy link
Member

Choose a reason for hiding this comment

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

Hmm auch net :]

@@ -107,7 +107,7 @@ public function testLogout()
{
$login = new rex_backend_login();
$login->setLogin($this->login, $this->password, false);
$this->assertTrue($login->checkLogin());
$this->assertFalse($login->checkLogin());
Copy link
Member

Choose a reason for hiding this comment

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

Ist das zufall oder hast du bewusst gerade die tests genommen zum testen die dann anschließend wg dem session header hart abbrechen?

Copy link
Member Author

Choose a reason for hiding this comment

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

ne, reiner zufall. Die tests brechen wegen dem session handler auch schon ohne diese änderung ab.

@staabm
Copy link
Member

staabm commented Jan 19, 2020

Depends on #3174

@staabm
Copy link
Member

staabm commented Jan 19, 2020

Ich wundere mich dass die test errors nur als annotation im checks tab anzegeigt werden, aber nicht im changes tab auftauchen

In https://github.com/mheap/phpunit-github-actions-printer Sieht es für mich so aus ala müsste es auch direkt am source angezeigt werden

@staabm
Copy link
Member

staabm commented Jan 19, 2020

Hmm wg der errors mal upstream reported
mheap/phpunit-github-actions-printer#4

@bloep
Copy link
Member Author

bloep commented Jan 19, 2020

Ich wundere mich dass die test errors nur als annotation im checks tab anzegeigt werden, aber nicht im changes tab auftauchen

In https://github.com/mheap/phpunit-github-actions-printer Sieht es für mich so aus ala müsste es auch direkt am source angezeigt werden

hab mich auch gewundert... Deswegen hatte ich den einen Testcase auch failen lassen, um zu sehen, ob die Marker nur bei geänderten Dateien dran kommen.

@staabm
Copy link
Member

staabm commented Jan 19, 2020

hab aktuell keine idee

@staabm
Copy link
Member

staabm commented Jan 19, 2020

(was ich mir noch vorstellen könnte: den fehler den wir sehen hatten wir schon vorher in der testsuite und er entsteht nicht durch die änderungen aus dem PR selbst)

@bloep
Copy link
Member Author

bloep commented Jan 19, 2020

(was ich mir noch vorstellen könnte: den fehler den wir sehen hatten wir schon vorher in der testsuite und er entsteht nicht durch die änderungen aus dem PR selbst)

das so oder so. Hab beides auch lokal bei mir aufm Rechner getestet. Meine Vermutung ist nach wie vor, dass der andere den unterdrückt. Ich schmeiß mal nen debugger an, ggf. kann ich damit sehen warum bei dem einen kein fehler kommt.

@staabm
Copy link
Member

staabm commented Jan 20, 2020

Es gibt ein neues release.. mal updaten und ausprobieren

@staabm staabm mentioned this pull request Jan 22, 2020
@staabm staabm closed this Jan 22, 2020
@gharlan gharlan deleted the bloep-patch-1 branch January 26, 2020 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Phpunit github action
2 participants