Sort control files before loading to allow results to be predictable

[mattray]

Fixes #3797

Signed-off-by: Matt Ray matthewhray@gmail.com

Description

Sorts the control files for predictable output. This is a frequent request, just submitting a PR for the simplest solution.

Related Issue

#3887
#3840

Types of changes

Simply sorts the control files.

Checklist:

• I have read the CONTRIBUTING document.

[codeclimate]

Code Climate has analyzed commit ce5a609 and detected 0 issues on this pull request.

View more on Code Climate.

[coveralls]

Coverage remained the same at 83.694% when pulling ce5a609 on mattray:sort_controls into 399b1cb on inspec:master.

[zenspider]

I'm against this at a philosophical level. Why is this needed?

[clintoncwolfe]

It seems straightforward, but my only concern is the lack of testing - not so much because we want to verify that it works, but because we want to discover how control file sorting works under other conditions - such as multiple profiles, on the same command line, inheritance, etc. Also, should this be documented?

[clintoncwolfe]

My understanding is that people want to have setup and teardown files - aaa.rb to initialize, zzz.rb to cleanup, etc.

[mattray]

I submitted the patch because the customer said they wanted consistent ordering of test results and this worked for them. If we document it as a feature, then we'll need to put a lot more effort into testing the scenarios @clintoncwolfe mentioned. Would it be acceptable to merge the patch and see if this is sufficient for the other issues that have been raised in the past and revisit it if it is not?

[zenspider]

Consistent ordering of test results is (or can be) different from consistent ordering of test execution.

I think this issue will require some extra discussion. I might need recalibration.

[miah]

It feels like this should be a reporter modification.

[aaronlippold]

In the end I think we would sort in the reporter even if we sorted on the input we would want to guarantee that the order was as expected coming out of the reporter. That's the interface point right.

…

On Wed, Jul 24, 2019, 6:05 PM Miah Johnson ***@***.***> wrote: It feels like this should be a reporter modification. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#4326?email_source=notifications&email_token=AALK42FGFMS3GVASS3K5AELQBDGZLA5CNFSM4IF7L25KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2XXXTY#issuecomment-514816975>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALK42H7VEMVQC7Y3ILNXETQBDGZLANCNFSM4IF7L25A> .

[clintoncwolfe]

@mattray can you clarify - what is wanted here? predictable execution order, or predictable results ordering?

[aaronlippold]

Hi, just my two cents. we've had this discussion before where people wanted a consistent execution order but that was in regards to them trying to use the output of a prior test in the input of a next test. This will and can lead to very unexpected results and not something we should encourage. In my opinion. Each test should be independent. The original ask if I remember correctly was that they wanted consistent ordered results output. Walking down the other path is dangerous in my opinion and will lead to many bad habbits.

…

On Thu, Jul 25, 2019, 12:00 PM Clinton Wolfe ***@***.***> wrote: @mattray <https://github.com/mattray> can you clarify - what is wanted here? predictable execution order, or predictable results ordering? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#4326?email_source=notifications&email_token=AALK42BCXLJ2B4S6XOYAWCLQBHEZVA5CNFSM4IF7L25KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2Z5TJY#issuecomment-515103143>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AALK42GO5H646KFPONGUTUDQBHEZVANCNFSM4IF7L25A> .

[zenspider]

After discussion and further consideration, I'm going to have to push back on this. It's dangerous (read: leads to false positives caused by test order dependencies) and not something we want to support.

I submitted the patch because the customer said they wanted consistent ordering of test results

This is a perfect description of the problem at hand. This should have become an issue asking for exactly that. What this PR is is a prescription of what you want, but it is the wrong solution for the actual problem at hand (a report order issue, not a run order issue). Sorry to have to go this way, but I have Very Strong Feelings™ about testing (and put out the very first test framework that actually randomized order to maintain exactly this).

[zenspider]

Also: Please do file that issue so we can get it on the board and you can track it.

[mattray]

@zenspider was #3797 not sufficient?

[zenspider]

@mattray I overlooked that. I would say that it is NOT sufficient for the reasons I explained above. Being descriptive as to what your problem is helps us find a solution that fits both the codebase, best practices, and the customer... this ticket is not that. It is prescriptive ("sort the controls to allow X"). I will poke at it.