mrtnzlml-archive / testbench

:green_apple: Simple integration testing tool for Nette applications
http://zlml.cz/jednoduche-testovani-pro-uplne-kazdeho
41 stars 13 forks source link

Post request na signal - httpRequest neobsahuje postparametry #21

Closed xklid101 closed 8 years ago

xklid101 commented 8 years ago

Pokud testuju POST request na signal, tak pri pouziti TPresenter::check(), kde se predaji vsechny parametry vcetne POST, uz do toho signalu post parametru nedoputuji. Je to tim, ze v TPresenter::check() vytvaris HttpRequestMock vzdy s prazdnym polem pro $post parametry. Je k tomu nejaky special duvod? Nebylo by lepsi predat tomu HttpRequestMocku parametry jake dostane methoda ::check()? Ja totiz v signalu taham post data z $httpRequest->getPost()...

Test pro issue:

/**
 * @testCase
 */
class Issue_HttpRequestPost extends TestCase
{
    use \Testbench\TPresenter;

    public function testHandleSignal()
    {
        $this->checkSignal('Presenter:default', 'signal', ['id' => 1], ['postparam' => 'postparamvalue']);

        $presenter = $this->getPresenter();
        $container = \Testbench\ContainerFactory::create(FALSE);

        $appRequest = $presenter->getRequest();
        $httpRequest = $container->getService('httpRequest');

        Assert::same('postparamvalue', $appRequest->getPost('postparam'));
        Assert::same('postparamvalue', $httpRequest->getPost('postparam'));
    }
}

(new Issue_HttpRequestPost())->run();
xklid101 commented 8 years ago

Ted jsem zjistil, ze to je naprosto stejne s GET parametry. Test je obdobny, muze vypadat nejak takto:

Test pro issue GET:

/**
 * @testCase
 */
class Issue_HttpRequestGet extends TestCase
{
    use \Testbench\TPresenter;

    public function testAction()
    {
        $this->checkAction('Presenter:default', ['getparam' => 'getparamvalue']);

        $presenter = $this->getPresenter();
        $container = \Testbench\ContainerFactory::create(FALSE);

        $appRequest = $presenter->getRequest();
        $httpRequest = $container->getService('httpRequest');

        Assert::same('getparamvalue', $appRequest->getParameter('getparam'));
        Assert::same('getparamvalue', $presenter->getParameter('getparam'));
        Assert::same('getparamvalue', $httpRequest->getQuery('getparam'));
    }
}

(new Issue_HttpRequestGet())->run();
xklid101 commented 8 years ago

offtopic: Jeste me napadlo, ze kdyz mas Tpresenter::checkRedirect(), tak tam kontrolujes regexp [a-z0-9?&=_/]* , ale pokud url obsahuje nejake urlencoded symboly, tak to neprojde. Ja si tam musim jako $path parametr davat napr. toto /[a-zA-Z0-9?&=_/%.-]*, tak jestli by nestalo zato, rozsirit ten regexp... ale vzhledem k tomu, ze si to do parametru muzu pridat sam, tak to k zivotu nepotrebuju.... No a jinak, kdyz uz ti tady "pridavam praci", tak ti alespon musim podekovat za Testbench, je to super a usetril jsi mi spoooooustu starosti s testovanim, tak dik ;)

mrtnzlml commented 8 years ago

Vyzkoušej prosím dev-master - jestli už je to tak, jak by mělo být.

Nepoužívej \Testbench\ContainerFactory::create(), k tomu je určená traita TCompiledContainer. Podívej se, jak jsem upravit ty testy tak, aby to bylo správně.

Jak myslíš ten regulární výraz? Ideálně můžeš prosím zase poslat minimální test? Regulár je vlastně jen na parametry té URL, takže máš problém spíš s tím?

xklid101 commented 8 years ago

Super, uz to u me prochazi i s pouzivanim httpRequest

A ten regexp, ano, jde hlavne o ty parametry url. Bud presemerujes s parametrem, nebo je v presenteru persistentni parametr. Udelal jsem ten test, ale je potreba upravit ten tvuj testovaci PresenterPresenter a to tak, ze se tam zada persistentni parametr (jinak nevim jak bych docilil presemerovani s parametrem v testu):

/**
 * @persistent
 */
public $persistentParameter;

a test je pak tady: (komentovany regexp v test metode je potreba pridat za lomitko aby to proslo)

/**
 * @testCase
 */
class Issue_RedirectPersistentParameter extends \Tester\TestCase
{

    use \Testbench\TPresenter;

    public function testRedirectPersistentParameter() {
        // [a-zA-Z0-9?&=_/%.+-]*
        $this->checkRedirect('Presenter:redirectRss', '/', ['persistentParameter' => 'Url-En.coded Value MixedCasě?!']);
        $this->checkRedirect('Presenter:redirect', '/', ['persistentParameter' => 'Url-En.coded Value MixedCasě?!']);
    }

}

(new Issue_RedirectPersistentParameter)->run();
mrtnzlml commented 8 years ago

Podívej se prosím znovu na dev-master, jestli už je to ok. Implementoval jsem to trošku jinak. Možná ti nějaký test failne, ale podívej se, jestli to není dobře. Předtím to nekontrolovalo správně nějaké případy přesměrování. Takže je to možná malinký BC break, ale je to vlastně dobře, protože to kontroluje něco navíc... :)

EDIT: Pokud to bude ok, tak vydám novou verzi.

xklid101 commented 8 years ago

par testu mi to rozbilo, ale jinak ok, upravil jsem to a vypada to lip nez predtim... na novou verzi jsem pripraven :)

jo a kdyz jsi zmenil to fake.url na $url = $url ?: new Http\UrlScript('http://test.bench/'); - tak jelikoz v moji app pocitam na jednom miste hash z cele url, a ten hash potrebuju vedet driv nez probehne nejaka kontrola (init presenteru v testu), rozbilo se mi to diky tomu i tam. Nic co bych nedokazal behem chvilky spravit, ale mozna by se mohlo to url pro testbench pridat do rozsireni, aby bylo pristupne napr. pres testovaci config.neon a pak pouzit toto misto toho 'hardcoded' pro HttpRequestMock...

mrtnzlml commented 8 years ago

To, že jsi musel něco upravovat ale dávalo smysl, je to tak? Jakože jsi kontroloval URL na lomítko, ale ve skutečnosti tam lomítko být nemá (nebo něco takového logického). To bych pak ani nepovažoval za BC break, protože to tak má být a je to spíš oprava chyby. Navíc to teď ukazuje pěkně:

screenshot from 2016-05-12 13 25 27

Nad tím URL v configu jsem také přemýšlel. Tvůj důvod je logický, takže se to ještě pokusím zapracovat. A pak vydám tu verzi.

xklid101 commented 8 years ago

J, presne jak pises... S tim BC breakem nevim. Predtim, kdyz se zadalo path, tak to testovalo tu zadanou path a ta pak mohla pokracovat jakkoliv dal, takze stacilo zadat treba jen zacatek '/admin' a matchlo to i '/admin/users', '/admin/accounts/3' apod. Kdezto ted to matchne pouze presne zadane url. Pokud se ovsem nepletu... Takze jestli to je BC break je asi vec nazoru a politickeho presvedceni :)

A URL v configu pro me neni uplne tak zasadni, ale kazdopadne to bude prijemne vylepseni.

mrtnzlml commented 8 years ago

Máš pravdu. Mění to chování, ale podle původních představ se to chovalo blbě a určitě to nebylo zamýšlené. Proto to zřejmě vydám jen jako minor verzi 2.2. Teď se to tedy chová tak jak by mělo s tím, že je možné tam poslat i regulární výrazy pro kontrolu cesty:

$this->checkRedirect('Presenter:redirectRss', '/x/y/rss');
$this->checkRedirect('Presenter:redirectRss', '/.*');
$this->checkRedirect('Presenter:redirectRss', '/(x|y)/(x|y)/.?s{2}');