Consistent header manipulation and testing #12079

Closed
nijel opened this Issue Mar 8, 2016 · 15 comments

Projects

None yet

2 participants

@nijel
Member
nijel commented Mar 8, 2016

Currently we need runkit to do testing of some header() calls and intentionally skip some of them when running in testsuite.

These should be replaced by Response::header() and tests should be adjusted to mock this method.

For example this has been already done for cookie auth in 2148e7c

@ragnerok
Contributor
ragnerok commented Nov 9, 2016

I would like to try fixing this . Can you give a few references ?
I am new to open source & contribution so any help will be appreciated !

@nijel
Member
nijel commented Nov 9, 2016

There is example commit linked in the original issue, I think it shows quite well what needs to be done.

@ragnerok
Contributor

Where are the files in which I need to do the changes ?
There are too many files in test directory .
Any help would be appreciated !

@nijel
Member
nijel commented Nov 30, 2016

Look for anything using $GLOBALS['header']:

  • test/classes/plugin/auth/AuthenticationSignonTest.php
  • test/libraries/PMA_Form_Processing_test.php
  • test/libraries/PMA_user_preferences_test.php
  • test/libraries/core/PMA_headerLocation_test.php
@ragnerok
Contributor
ragnerok commented Dec 3, 2016

ok working on it

@ragnerok
Contributor
ragnerok commented Dec 4, 2016

So all I have to do is

  1. create a new response for each $GLOBALS['header']
  2. then do something like this $response->header($GLOBALS['header'])

Is this correct or do I have to do something else or something more ?

@nijel
Member
nijel commented Dec 7, 2016

No, it's not only about tests.

  1. The code which is being needs to be replace header() call with $response->header() (see here)
  2. Remove runkit dependency from the test (here)
  3. Add mocking of Response object methods to the test (here and here
  4. Remove assertion on $GLOBALS['header'] as this is already done in mocks above, see here
    and then the test s
@ragnerok ragnerok added a commit to ragnerok/phpmyadmin that referenced this issue Dec 7, 2016
@ragnerok ragnerok Implemented Header auth
Fixes #12079
Signed-off-by: Osaid <osaid.nasir@gmail.com>
0276c9c
@ragnerok
Contributor

@nijel for this assertion how do we replace it with mock of response object ?

@nijel
Member
nijel commented Dec 14, 2016

You can use withConsecutive for checking multiple calls, see http://stackoverflow.com/a/23942979/225718

@ragnerok
Contributor

also here and here i think we should remove the checks for PMA_TEST_HEADER since they have been defined now

@nijel
Member
nijel commented Dec 20, 2016

Yes, there are probably more places where this has to be adjusted.

@nijel nijel added a commit that referenced this issue Dec 20, 2016
@ragnerok @nijel ragnerok + nijel Use Header class for headers manipulation
* Added mocking of response object
* Rectified mistake of calling method once
* implemented mock properly
* replaced header with response header 

Issue #12079

Signed-off-by: Osaid osaid.nasir@gmail.com
73c6a7e
@ragnerok ragnerok added a commit to ragnerok/phpmyadmin that referenced this issue Dec 21, 2016
@ragnerok ragnerok Update from original (#2)
* Do not delete session on fatal error

I see no reason why this should be done, the fatal error is used
in following cases:

* Very early when there is no session (eg. missing extension)
* Invalid value for parameters
* Invalid invocation like too big request

In neither case session removal will do any good.

Signed-off-by: Michal Čihař <michal@cihar.com>

* Do not use control link when working with stored procedures

Fixed stored procedure execution.

Fixes #12813

Signed-off-by: Michal Čihař <michal@cihar.com>

* Fix early fatal errors

We can not rely on whole stack being ready.

Fixes #12810

Signed-off-by: Michal Čihař <michal@cihar.com>

* Fix display of custom header and footer in certain edge cases.
Issues #12801 and #12802

Signed-off-by: Isaac Bennetch <bennetch@gmail.com>

* Translated using Weblate (Dutch)

Currently translated at 100.0% (3222 of 3222 strings)

[CI skip]

* Translated using Weblate (Dutch)

Currently translated at 100.0% (3239 of 3239 strings)

[CI skip]

* Translated using Weblate (French)

Currently translated at 100.0% (3222 of 3222 strings)

[CI skip]

* Translated using Weblate (French)

Currently translated at 100.0% (3239 of 3239 strings)

[CI skip]

* Translated using Weblate (French)

Currently translated at 100.0% (3222 of 3222 strings)

[CI skip]

* MySQL allows precision to be specified for DATETIME, TIME type fields too

Fix #12814

Ref : http://dev.mysql.com/doc/refman/5.7/en/fractional-seconds.html

Signed-off-by: Deven Bansod <devenbansod.bits@gmail.com>

* ChangeLog for #12814

Signed-off-by: Deven Bansod <devenbansod.bits@gmail.com>

* Share code for cascading ajax flag

Signed-off-by: Michal Čihař <michal@cihar.com>

* Get response instance just once

Signed-off-by: Michal Čihař <michal@cihar.com>

* Register shutdown directly on singleton instance

This avoids additional call in the shutdown handler.

Signed-off-by: Michal Čihař <michal@cihar.com>

* Simplify checking for ajax request

Signed-off-by: Michal Čihař <michal@cihar.com>

* Include token in all filter requests

Fixes #12786

Signed-off-by: Michal Čihař <michal@cihar.com>

* Fix documentation markup

Signed-off-by: Isaac Bennetch <bennetch@gmail.com>

* Hints about using Composer for library dependencies

Signed-off-by: Isaac Bennetch <bennetch@gmail.com>

* Use documentation link instead of wiki for Composer details

Signed-off-by: Isaac Bennetch <bennetch@gmail.com>

* Uncomment mistakenly commented Selenium testing setting

Signed-off-by: Deven Bansod <devenbansod.bits@gmail.com>

* Translated using Weblate (Spanish)

Currently translated at 100.0% (3222 of 3222 strings)

[CI skip]

* Translated using Weblate (Spanish)

Currently translated at 99.8% (3234 of 3239 strings)

[CI skip]

* Remove .htaccess from tests

Issue #12348

Signed-off-by: Michal Čihař <michal@cihar.com>

* Add source release support

See #12348

Signed-off-by: Michal Čihař <michal@cihar.com>

* Share code for rendering custom header and footer

Fixes #12802

Signed-off-by: Michal Čihař <michal@cihar.com>

* Fix empty password login for http authetication

Fixes #12828

Signed-off-by: Michal Čihař <michal@cihar.com>

* Honor user configured connection collation

* the user set collation is now honored
* default value has been changed to utf8mb4
* it is downgraded to utf8 if server does not support it

Fixes #12826

Signed-off-by: Michal Čihař <michal@cihar.com>

* Fix HTTP auth test expectations

Signed-off-by: Michal Čihař <michal@cihar.com>

* Remove default value for js parameter

The null is there implicitely anyway.

Fixes #12829

Signed-off-by: Michal Čihař <michal@cihar.com>

* Add some missing html encoding

Issue #12804

Signed-off-by: Michal Čihař <michal@cihar.com>

* Feature: #12069: Filtering tables on table listing of particular database.

Signed-off-by: Yunus Çağrı <ycagriyurdakul@gmail.com>

* Feature: #12069: Filtering tables on table listing of particular database.

Signed-off-by: Yunus Çağrı <ycagriyurdakul@gmail.com>

* Determine whether to use openssl just once

Issue #12293

Signed-off-by: Michal Čihař <michal@cihar.com>

* Correctly report OpenSSL errors from cookie encryption

Without calling openssl_error_string() we pollute openssl global state
and some other library might report this as failure (eg. mysqlnd driver
when connecting to SSL enabled server).

Fixes #12293

Signed-off-by: Michal Čihař <michal@cihar.com>

* Use same encryption key with openssl and phpseclib

Issue #12293

Signed-off-by: Michal Čihař <michal@cihar.com>

* Test both with and without phpseclib

This ensures we generate compatible data in both cases.

Issue #12293

Signed-off-by: Michal Čihař <michal@cihar.com>

* Use Header class for headers manipulation

* Added mocking of response object
* Rectified mistake of calling method once
* implemented mock properly
* replaced header with response header 

Issue #12079

Signed-off-by: Osaid osaid.nasir@gmail.com

* Inline javascript codes are removed. Each is used for table search

Signed-off-by: Yunus Çağrı <ycagriyurdakul@gmail.com>

* Update db_structure.js

* Tab spaces are removed.

Signed-off-by: Yunus Çağrı <ycagriyurdakul@gmail.com>

* Unbind event on page unload

Issue #12838, #12069

Signed-off-by: Michal Čihař <michal@cihar.com>

* Use existing filtering template for table filtering

Issue #12838, #12069

Signed-off-by: Michal Čihař <michal@cihar.com>

* Changelog for issues #12838 and #12069

Signed-off-by: Michal Čihař <michal@cihar.com>
a88ed64
@ragnerok
Contributor

similar problem here and here

@ragnerok
Contributor
ragnerok commented Jan 8, 2017

are there any other files that need to be changed ?

@nijel nijel added a commit that referenced this issue Jan 8, 2017
@nijel nijel Use shared response mock in cookie auth tests
Issue #12079

Signed-off-by: Michal Čihař <michal@cihar.com>
3c2867c
@nijel
Member
nijel commented Jan 8, 2017

I think we're pretty much done here, thanks!

However there is still one thing to improve - as you've introduced generic method for doing these tests, existing manual code could be migrated to it:

  • test/classes/plugin/auth/AuthenticationHttpTest.php
  • test/classes/plugin/auth/AuthenticationSignonTest.php
  • test/libraries/PMA_Form_Processing_test.php
  • test/libraries/PMA_user_preferences_test.php
  • test/libraries/core/PMA_headerLocation_test.php

I've done so for cookie authentication in 3c2867c, but I will not get to others in next week...

@nijel nijel added a commit that referenced this issue Jan 8, 2017
@nijel nijel Remove code for PMA_TEST_HEADERS
It's no longer necessary.

Issue #12079

Signed-off-by: Michal Čihař <michal@cihar.com>
8633f44
@nijel nijel self-assigned this Jan 8, 2017
@nijel nijel added this to the 4.7.0 milestone Jan 8, 2017
@ragnerok
Contributor

I think this should be closed now

@nijel nijel added a commit that closed this issue Jan 18, 2017
@nijel nijel Use shared mock for response without any header
Fixes #12079

Signed-off-by: Michal Čihař <michal@cihar.com>
ce9f84e
@nijel nijel closed this in ce9f84e Jan 18, 2017
@nijel nijel added a commit that referenced this issue Jan 18, 2017
@nijel nijel Share more code for reponse mocking
Issue #12079

Signed-off-by: Michal Čihař <michal@cihar.com>
028f32d
@nijel nijel added a commit that referenced this issue Jan 18, 2017
@nijel nijel Share more of response mocking
Issue #12079

Signed-off-by: Michal Čihař <michal@cihar.com>
815149c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment