IndentationCheckTest makes assumption about the order of HashMap #3369

Closed
azy2 opened this Issue Jul 25, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@azy2
Contributor

azy2 commented Jul 25, 2016

The tests IndentationCheckTest.testGetRequiredTokens and IndentationCheckTest.testGetAcceptableTokens both use handlerFactory.getHandledTypes which internally uses a HashMap. HashMap does not guarantee any order of iteration. In these tests however, the code is assuming that the order of iteration will be the same twice in a row, which can result in test failures.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 26, 2016

Member

@azy2 , good catch.
How did you reproduce this problem ?
I need this to setup a test on CI avoid such problem in future.

Member

romani commented Jul 26, 2016

@azy2 , good catch.
How did you reproduce this problem ?
I need this to setup a test on CI avoid such problem in future.

romani added a commit that referenced this issue Jul 26, 2016

@romani romani added this to the 7.1 milestone Jul 26, 2016

@azy2

This comment has been minimized.

Show comment
Hide comment
@azy2

azy2 Jul 26, 2016

Contributor

I caught these assumptions with a tool called NonDex which I helped develop with a research group at the University Of Illinois. The tool is available as a maven plugin from Maven Central and it is pretty easy to add to your pom and Travis. If you want I can create a pull request integrating it.

Contributor

azy2 commented Jul 26, 2016

I caught these assumptions with a tool called NonDex which I helped develop with a research group at the University Of Illinois. The tool is available as a maven plugin from Maven Central and it is pretty easy to add to your pom and Travis. If you want I can create a pull request integrating it.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 27, 2016

Member

@azy2 , please create PR to integrate it, we like static analysis tools, we have bunch of them already enforced over our code.

Member

romani commented Jul 27, 2016

@azy2 , please create PR to integrate it, we like static analysis tools, we have bunch of them already enforced over our code.

@azy2

This comment has been minimized.

Show comment
Hide comment
@azy2

azy2 Jul 27, 2016

Contributor

@romani I added the pull request to integrate NonDex

Contributor

azy2 commented Jul 27, 2016

@romani I added the pull request to integrate NonDex

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 29, 2016

Member

I am closing this issue, all further updates should be done at #3378

Member

romani commented Jul 29, 2016

I am closing this issue, all further updates should be done at #3378

@romani romani closed this Jul 29, 2016

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