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

Return correct type on purge when limit has been reached. #185

Open
wants to merge 4 commits into
base: 7.x-1.x
Choose a base branch
from

Conversation

nedSaf
Copy link

@nedSaf nedSaf commented Apr 3, 2017

@nedSaf
Copy link
Author

nedSaf commented Apr 3, 2017

Added a test, before the fix:
running_tests___drupal_7_testing

After the fix:
test_result___drupal_7_testing

@nedSaf
Copy link
Author

nedSaf commented Apr 3, 2017

@RoySegall Ready for code review.

@@ -586,6 +586,45 @@ class MessageCron extends DrupalWebTestCase {
}

/**
* Test purging one message type when there're messages more than the cron

Choose a reason for hiding this comment

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

First line of the php doc block need to in a single line.

@RoySegall
Copy link

What the settings of your installation? I've set up a message type with purge limit of 2 and set the global purge to 10(like in the tests) and created 30 messages and the cron ran OK.

@nedSaf
Copy link
Author

nedSaf commented Apr 3, 2017

@RoySegall I ran the tests on a clean installation.

@RoySegall
Copy link

I don't think so because when running the tests a clean env. is created for the tests.

@nedSaf
Copy link
Author

nedSaf commented Apr 3, 2017

I don't understand, let's discuss when you have time.

@nedSaf
Copy link
Author

nedSaf commented Jul 20, 2017

@RoySegall Fixed your comment, so you are saying that you are running this test on master (7.x-1.x) and it's passing?

@RoySegall
Copy link

Back then - yes. I can test it again.

@nedSaf
Copy link
Author

nedSaf commented Jul 20, 2017

@RoySegall Please :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants