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

Support expected-stdout, expected-stderr by default #27

Merged

Conversation

michaljurecko
Copy link
Contributor

@michaljurecko michaljurecko commented Sep 9, 2020

Solving: https://keboola.atlassian.net/browse/COM-203

Problem 1:

  • V komponentach casto vyuzivame datadir-testy, no potrebujeme kontrolovat aj stdout (pri sync akciach) a stderr (pri chybach)
  • Podpora pre stdout a stderr je implementovana v DatadirTestSpecification, ale chybala defaultna implementacia expected-stderr a expected-stdout.
  • Teda v kazdom projekte sme doteraz museli prepisovat metodu processOneTest:

Problem 2:

  • stdout a stderr potrebujeme porovnavat cez assertStringMatchesFormat nie assertSame
  • vo vystupe sa skor vzdy objavi nejake IDcko, ktore je pri kazdom behu ine
  • alebo nejaky prefix, ktory je závislý na verzii databazy a testujeme to oproti viacerym verziam, a pod.
  • ... ak sa pouzije assertStringMatchesFormat, tak v expected-stderr mozeme pouzit placeholdery, napr %a.

@michaljurecko michaljurecko force-pushed the webrouse-COM-203-support-expected-stdout-stderr branch from b4c84da to c299f26 Compare September 9, 2020 09:01
$this->assertSame(
$specification->getExpectedStdout(),
$runProcess->getOutput(),
$this->assertStringMatchesFormat(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

stdout a stderr porovnavame cez assertStringMatchesFormat.
Okrem toho je tu pridany aj trim, kedze synchronne akcie casto neobsahuju \n na konci vystupu a IDEcko obycajne prida \n do expected-stdout a potom sa to tazko debuguje, preco to spadlo.

@michaljurecko
Copy link
Contributor Author

@zajca riesil uz skor stderr ale z nejakeho dovodu sa to nemerglo:
#21

@michaljurecko michaljurecko marked this pull request as ready for review September 9, 2020 09:07
@ondrajodas
Copy link
Contributor

cca za hoďku na to kouknu

Copy link
Contributor

@ondrajodas ondrajodas left a comment

Choose a reason for hiding this comment

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

lgtm

@michaljurecko michaljurecko merged commit 72bbbab into master Sep 9, 2020
@odinuv odinuv deleted the webrouse-COM-203-support-expected-stdout-stderr branch September 9, 2020 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants