Skip to content

Conversation

IrAlfred
Copy link
Member

Related issue #306

@IrAlfred IrAlfred requested a review from kroky October 11, 2025 23:45
@kroky
Copy link
Member

kroky commented Oct 13, 2025

@IrAlfred thanks, it looks good but can you please add some unit/integration tests to cover the new functionality?

@IrAlfred IrAlfred force-pushed the integrate-advanced-search-into-saved-searches branch 2 times, most recently from 77eebbe to abce26c Compare October 14, 2025 13:28
@IrAlfred IrAlfred requested a review from kroky October 14, 2025 13:45
@IrAlfred IrAlfred force-pushed the integrate-advanced-search-into-saved-searches branch from abce26c to afa4d24 Compare October 14, 2025 21:32
public function setUp(): void {
require_once __DIR__.'/../../bootstrap.php';
require_once APP_PATH.'modules/core/modules.php';
require_once APP_PATH.'modules/saved_searches/modules.php';
Copy link
Member

Choose a reason for hiding this comment

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

These three lines are inappropriately placed here. They should be part of the test suite bootstrap process.

)
);

$this->saved_searches = new Hm_Saved_Searches($this->sample_data);
Copy link
Member

Choose a reason for hiding this comment

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

If you initialize this here, why do you keep initializing it in individual tests and not just reuse?

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