testsuite: Ensure that config.{msys,cygwin} are initialized
ClosedPublic

Authored by bgamari on Aug 10 2018, 9:44 AM.

Details

Reviewers
monoidal
bgamari created this revision.Aug 10 2018, 9:44 AM

What is the motivation?

I would prefer not to do this, since currently cygwin and msys are initialized once in runtests.py. This way, if the code reads those variables before they're initialized, it throws an exception.

(It would be better if we initialized values only in __init__ and not mutated them later, but making this change is not trivial)

What is the motivation?

The motivation was consistency. During a read of the driver it struck me as odd that these weren't initialized, while the other fields were.

I would prefer not to do this, since currently cygwin and msys are initialized once in runtests.py. This way, if the code reads those variables before they're initialized, it throws an exception.

Couldn't you raise this same objection about any of the other surrounding fields?

(It would be better if we initialized values only in init and not mutated them later, but making this change is not trivial)

It seems to me like moving the msys detection logic into __init__ would likely be fairly straightforward.

monoidal accepted this revision.Aug 27 2018, 8:35 AM

OK.

Yes, the same objection could be raised about other fields. My opinion is that in the end we shouldn't be initializing any of those to dummy values if they can be initialized correctly.

I agree that msys detection could be moved to __init__ easily; I meant that doing the proper initialization in __init__ of everything is nontrivial. Maybe I'll get to this one day.

This revision is now accepted and ready to land.Aug 27 2018, 8:35 AM
bgamari closed this revision.Jan 20 2019, 8:09 PM

Moved to GitLab.