Skip to content

"Description" should freely allow test names containing "(" and ")". #327

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

Open
aknrdureegaesr opened this issue Sep 25, 2011 · 2 comments
Open

Comments

@aknrdureegaesr
Copy link

The sad story

Like many other people, I have my own naming requirements for parametrized tests. I call my tests “data driven”. My test method names are a low level test implementation detail, not helpful to differentiate individual tests.

I solved my naming needs through a patched version of the Parameterized class and its TestClassRunnerForParameters inner class. This worked quite nicely at first. Until “(” and “)” appeared in the names of my tests.

As I found out the hard way, class Description has logic about what “(” and “)” mean in a test name. The assumptions made by that class are just plain wrong in my case. Also not good, the constructor of Description is private. So to get what I want, I cannot resolve to the usual "derive and change judiciously".

My request:

The present Javadoc promises:

Descriptions ... are entirely informational. They contain no logic aside from counting their tests.”

In view of that Javadoc, the present Description code is buggy.

Please make good on that promise. In my opinion, Descriptions should provide access to some strings that meaningfully identify the particular test or test suite to the end user - with no logic attached to those strings.

Please remove all assumptions from the code that description strings can be parsed to make available class and/or method names.

Workaround

I replace “(” to “[” and “)” to "]” .

(I will present these tests to a meeting of developers in a few weeks time, and those names will look ugly .)

@schaarsc
Copy link

schaarsc commented Sep 8, 2012

since this issue was referenced in #496 I think it makes sense to document the key statements from the discussion here.

summary (@kcooney please correct me if I'm wrong)

  • Description is part of the public api and any change should be backward compatible and not break existing code
  • people may use the (undocumented) feature, that by passing a name matching METHOD_AND_CLASS_NAME_PATTERN into createSuiteDescription a Test is created instead of a Suite

from my point of view we have the following options

  1. close this request with "wontfix"
  2. take the risk of breaking existing code and do not make any assumptions about Suite name
  3. modify Description as follows
    • add new fields: className, methodName
    • modify createSuiteDescription such as, that name is parsed immediately and className, methodName is filled if name matches, "normal" displayName is set otherwise
    • modify createTestDescription to fill className, methodName
    • adapt getters if affected by this change

from my previous discussions I assume that 2. is out of the question.
does 3. have any chance to get admitted? If so, I'd volunteer to try to provide a patch in that direction

@dsaff
Copy link
Member

dsaff commented Sep 10, 2012

@schaarsc, I worry you're conflating at least two bugs/requests that happen to touch on the same concepts. I am 100% behind the main thrust of this original bug, as I read it: in JUnit's own code, we should not make assumptions about what the "meaning" of the string in a Description is. I wrote Description#getMethodName and Description#getClass, and it was a terrible, terrible idea. We should deprecate those methods, and find new ways to do anything in the current code base that uses them.

Your #3 seems to go beyond this, in order to solve an additional problem, which I'm only partially understanding in #496.

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

No branches or pull requests

4 participants