Add a warning if a diff/patch file is selected and issue status is not testable

I certainly agree with this feature request! There are so many tweaky things about this, and this is one we can deal with fairly easily.

Another reason a test can be ignored: Has a name like -d6.patch. I'm going to make that name more descriptive though and should solve that one.

Great ideas.

Here, of course, it's the .d8.patch that causes it to be ignored. That's the (weird) formula for ignoring patches. We have to change it to something more obvious.

It *was*. Can't find it. Totally bizarre IMO.

Moving to PIFT, where it belongs.

@alexdrupaldev when you have a problem like that give us a link to the problem patch so we can explore it.

I also find it really annoying when it just says "ignored". Today I was debugging a problem which was caused by no test being inserted into the database. Same stuff. "Ignored".

Agreed. Would be nice to have a comment on WHY it's ignored AND let the user EDIT the previous post to switch it to "needs review". I can edit the comment but not the settings, so have to add additional posts which leads to the bloat and are uneccessary.

@rfray I also ran into the same problem as @alexdrupaldev. Also my first patch contribution to core: #806992: Run-on sentence in authorize.inc comment #13. Any ideas how to properly create a patch for core? Should I be running git diff with the --no-prefix flag?

Ooops. I failed to set the correct status flag. Please ignore comment 12.

Here's a first step ... should identify when issues are ignored due to status, and say so in the 'ignore' message.

+++ b/pift.pages.inc
@@ -188,7 +188,13 @@ function theme_pift_attachments(array $files, $closed) {
+        $row[] = '<em>' . t('Ignored:  Current issue status is not enabled for testing.');

How about saying even more helpful "Only issues in states 'needs review' or 'rtbc' get tested" ?

Also note that variable pift_test defaults to array(8, 14), not an empty array.

But I like this as a first step.

Can we rejigger the "ignore on weird filename" behavior in this patch, and make sure to give feedback on it?

Both the pift_status array and labels used for issue statuses are drupal.org dependent, and may not apply on a non-d.o site. While I admit that d.o is the prime use case, we shouldn't hard-code any assumptions.

How about simply 'Ignored: Check issue status.'?

I'm ok with that but the default is in fact already wired in by the install process even though it's not in the code. The variable empty case never happens because the var is set in the install if I'm not mistaken. But it's ok. We have a ways to go with this one anyway.

Reworded the 'Ignored' message.

This was a 'low-hanging fruit' fix, in that the information required to check the issue status was already available in the functions which call the 'theme' function in which the ignored message is set. This isn't really the case for the other potential 'ignored' reasons.

There is a ways to go on this particular issue ... but I don't see any reason why we would need to wait until we've fixed 'all' ignored states before committing a quick hitter like this in the meantime.

I didn't test this on the test environment (we could fiddle with pift_test to do that - you probably already have) but it looks good to me.

Unless.. you think we should just refactor to come up with a comprehensive approach to this.

I think we should commit this, but leave the issue open for a more comprehensive refactoring.

Committed the 'Ignored: check issue status' update in #18 to 6.x-2.x (3735e61).

Setting issue back to 'active' for followup on other ignore reasons.

Following this topic - such an annoying issue!

With #1171958: Allow files to be assigned to branch(es)/version(s) and thus tested against it, there is now a check in JS for a diff/patch file being selected. This would be a good place to put in a warning if the issue status isn't in a testable state.