Project

General

Profile

Actions
Copy link

Patch #32927

closed

CSS selector in test_index_should_show_warning_when_no_workflow_is_defined is too specific

Added by Vincent Robert over 5 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Low
Assignee:
Go MAEDA
Category:
Code cleanup/refactoring
Target version:
4.2.0
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Hi
Just a tiny patch which makes a test less specific. This way, a plugin can add columns to the table without breaking core tests.
Thank you

Files

more_generic_test.diff (711 Bytes) more_generic_test.diff Vincent Robert, 2020-01-30 14:24
Actions
Copy link
 #1

Updated by Go MAEDA over 5 years ago

Plugins often break core tests. Should we relax tests every time a plugin the test suite?

Actions
Copy link
 #2

Updated by Vincent Robert over 5 years ago

No, we should not ;) But I think this specific test is too precise. It will break as soon as the table is modified, even if the tested function has not changed.

But feel free to close my issue if you prefer to not update the current tests.

Actions
Copy link
 #3

Updated by Go MAEDA over 5 years ago

I am not in favor of accepting this patch as it is for the following reasons:

  • If the patch is accepted, many plugin developers may request to loosen tests only to fix test failures caused by their plugins
  • There are some other ":nth-of-type" in the test suite. Changing only one place makes the test inconsistent

However, I think it is OK to change "td:nth-of-type(3)" to "td.CLASS_NAME" instead of loosening the tests.

Actions
Copy link
 #4

Updated by Go MAEDA over 5 years ago

Go MAEDA wrote:

I am not in favor of accepting this patch as it is for the following reasons:

  • If the patch is accepted, many plugin developers may request to loosen tests only to fix test failures caused by their plugins
  • There are some other ":nth-of-type" in the test suite. Changing only one place makes the test inconsistent

However, I think it is OK to change "td:nth-of-type(3)" to "td.CLASS_NAME" instead of loosening the tests.

I have changed my mind.

I just noticed that the message to test can be specified by span.icon-warning. Now I think the current code is too strict and there is no problem to merge the suggested patch.

Actions
Copy link
 #5

Updated by Go MAEDA over 5 years ago

  • Target version changed from 4.0.7 to 4.2.0
Actions
Copy link
 #6

Updated by Go MAEDA over 5 years ago

  • Subject changed from Tiny patch which make a test less specific - Make easier for plugins to not break it to CSS selector in test_index_should_show_warning_when_no_workflow_is_defined is too specific
  • Status changed from New to Closed
  • Assignee set to Go MAEDA

Committed the patch. Thank you.

Actions
Copy link

Also available in: Atom PDF