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

[ticket/11608] add functional tests for search #1481

Merged
merged 24 commits into from Oct 7, 2013

Conversation

dhruvgoel92
Copy link
Contributor


public function test_sphinx()
{
$this->markTestIncomplete('Sphinx search not running for the test board');
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be wrapped by if (class_exists('SphinxClient')) or not? Then the tests would run, when it is available?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also the message should then be something like "Did not run Sphinx Search Tests, as Sphinx is not installed"

Copy link
Contributor

Choose a reason for hiding this comment

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

Also is this not catched by self::markTestSkipped("Search backend is not supported/running"); ?

@nickvergessen
Copy link
Contributor

please split things up into multiple files:

  • one containing all the test functionality
  • one for each backend extending the first file.

@dhruvgoel92
Copy link
Contributor Author

I have been unable to create tests for sphinx because sphinx indexer needs to be executed after we create index through acp. One possible way is to use exec ( ) but it is not recommended. If we cant come up with a solution rest of the tests should be merged IMO.

$crawler = self::request('GET', 'search.php?keywords=' . $keywords);
$this->assertEquals(0, $crawler->filter('.postbody')->count());
$split_keywords_string = str_replace(array('+', '-'), ' ', $keywords);
$this->assertEquals($split_keywords_string, $crawler->filter('#keywords')->attr('value'));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test currently passed only for mysql_fulltext.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And it should also pass for Postgres, not tested it though :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Travis is passing for postgres fulltext. Sphinx test are skipped anyway, so I am thinking of fixing native_Search since it is the one which is failing

@dhruvgoel92
Copy link
Contributor Author

#1506

@bantu
Copy link
Collaborator

bantu commented Jul 6, 2013

Please consider using tests/functional/search/*_test.php instead of tests/functional/search_*_test.php.

public function setUp()
{
parent::setUp();
$this->search_backend = 'phpbb_search_fulltext_mysql';
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be a string that never changes. Remove this method and assign the string at variable definition?

@bantu
Copy link
Collaborator

bantu commented Jul 22, 2013

Description says we are waiting for sphinx stuff here. Is this still the case? Please get this merge-ready without sphinx otherwise.

@dhruvgoel92
Copy link
Contributor Author

Merge ready. #1054 needs to be merged before this PR as this includes test for that test case.

@nickvergessen
Copy link
Contributor

Sphinx test fail... mark it as incomplete?

@dhruvgoel92
Copy link
Contributor Author

Fixed. The failure in Postgres is fixed in #1054

/**
* @group functional
*/
abstract class phpbb_functional_search_base_test extends phpbb_functional_test_case
Copy link
Contributor

Choose a reason for hiding this comment

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

should not end with testotherwise phpunit unneccessarily tries to run them

@dhruvgoel92
Copy link
Contributor Author

Fixed, can be merged now?

@nickvergessen
Copy link
Contributor

Can this be rebased?

Keyword search in search functional tests should be
performed as guests rather than logged in as admin.

PHPBB3-11608
Sphinx search for the test board cannot be tested

PHPBB3-11608
Search tests check for highlighted words in search results

PHPBB3-11608
Tests for each search backend are into their own separate files.
These separate classes inherit from a common search test case class.

PHPBB3-11608
Rename search_found and search_not_found to assert_search_found and
assert_search_not_found. Count .errorbox incase the search backend is not
supported and skip tests

PHPBB3-11608
Rename base class to phpbb_functional_search_base and fix Docblocks as per
phpbb guidelines.

PHPBB3-11608
EXreaction added a commit that referenced this pull request Oct 7, 2013
[ticket/11608] add functional tests for search
@EXreaction EXreaction merged commit 1e2ceb4 into phpbb:develop Oct 7, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants