quattor / maven-tools

Maven-based Build Tools
www.quattor.org
Apache License 2.0
3 stars 12 forks source link

Mocked FileWriter: set noaction=1 by default #150

Closed jouvin closed 7 years ago

jouvin commented 7 years ago

Also:

Fixes #139.

jouvin commented 7 years ago

Has been verified to properly work with CAF.

jouvin commented 7 years ago

Test is failing because I missed a problem when I ran the test locally as I tend not to run the mvn clean: the compilation of the profile (with use Test::Quattor qw(quattor)) seems to use the mocked FileWriter/Editor that has now noaction set by default instead of using the unmocked FileWriter... I have not been able to figure out where this is done. @stdweird any hint?

stdweird commented 7 years ago

@jouvin i'm trying to make the unittests of the ncm-network cleanup adhere to proper NoAction usage (by setting $Test::Quattor::NoAction=1), but i'm running in a lot of small mostly unrelated problems. this needs more testing first.

jouvin commented 7 years ago

@stdweird sure it needs to be tested with the various pieces. I think that if we manage to do it, at the end the test framework will be cleaner as there will really be no need to have CAF::Object::NoAction=1 defined in so many places...

jouvin commented 7 years ago

Problem mentioned in https://github.com/quattor/maven-tools/pull/150#issuecomment-290168019 fixed. Should be ready for testing with other Quattor components now...

stdweird commented 7 years ago

@jouvin i'm adapting and cleaning up the test framework for this case. it's a lot more complicated than this patch, but the new FileWriter allows for cleaner/fewer code, so i'd prefer not to merge this yet. i'll also try to clarify the make_directory issue in the PR.

jouvin commented 7 years ago

No problem, I always had in mind that this PR required careful evaluation and testing...

jrha commented 7 years ago

@jouvin if you have time could you rebase and resolve the conflicts?

stdweird commented 7 years ago

@jrha @jouvin unless i'm mistake, this PR was replaced by #160

jrha commented 7 years ago

It does indeed seem that way.