it is the duty of the configuration helper to build a map
providing the configuration. Having facilites to use the
ConfigurationHelper as some kind of configuration registry
makes life more complicated.
there is only one test class making use of it left. This test class is the
GitUtilsTest itself.
Hence moving this member downwards in the test class hierarchy into GitUtilsTest.
I doubt it makes sense to make use of a generic git utils mock somewhere else since this basically
means to test the GitUtils class in the conxtext of other examinees. For that there is no need.
The GitUtils class should be tested by the corresponding test class, but not in a transive manner
by other tests. For all other tests a specific git utils mock should be provided mocking the corresponding
method class. This is already the case today, otherwise we would have more test classes making use
of the generic git utils mock.
- In ArtifactSetVersionTest the method was declared,
but not used at all.
- In MtaUtilsTest there was a particular Object created
for the sole purpose of serving as script.
For that we have the nullScript.
- In DockerArtifactVersioningTest 'this' was configured to
serve as script. There is basically no reason why the
instance of the JUnitTest should serve as script.
Instead we have the nullScript for that purpose.
GStrings might be handed over e.g. via signature to steps.
GStrings in configuration makes it harder to deal with configurations.
E.g.
```if(param == 'a' || param == 'b')``` returns true if a is a GString representing 'a'
but
```if(param in ['a', 'b'])``` returns false.
It would be possible not to use the ```in``` notation in our code. But this
increases readability.
GString are not significant and can be replaced by the corresponding java.lang.String
representation without loss of information. Hence it is justified IMO to ensure
there are no GStrings contained in the configuration map.
if we don't mock the GitUtils here changes inside GitUtils affecting the git commands
issued at the command line requires changes here. In fact we have to react on
implementation details of the GitUtils here. It is better to be independent from that
implementation details here since this leads to more focused and smaller commits.
- there is no need to do the git commit inside the body of sshAgent
- there is no need to do the git tag inside the body of sshAgent
- side catch: availablility of git user name and git user email is
checked at its own. If one is missing the other may be present
~somehow~ in the configuration.
When we register a closure as file, the closure will be exectutd.
Otherwise we return what is registered.
This gives us a maximum level of flexibility. We can throw exceptions (e.g. FileNotFound) as
as test setup requires this, in simple cases we provide the yaml as a string.
We can now registering files to JenkinsReadYamlRule by
providing the file name alongside with the expected content
(or e.g. an expception)
With that change it is possible to remove pwd statements
from mtaBuild. These statements was used in order to pass
a temporary directory inside the mtaBuild (code under test).
This is not needed anymore since we can register the files
directly.
Having pwd implies working with absolute pathes which is
also a no-go when working with docker, since the absolute
pathes inside and outside docker are normally not the same.
For pathes relative to a build root directory it is rather
easy to keep the pathes consistent the same.
Adjust sources according to registering yaml file to jenkins rule.
For mtaBuild this means also: get ride of absolute pathes for denoting the yaml file.
Having absolute pathes makes it difficult/impossible to work also with dockerized versions
of mtaBuild since the absolute pathes are most probably not the same inside and outside
the docker container, but the relatives pathes can be kept the same easily.
up to now we check some values provided by the DefaultConfigurationCache. This is some kind
of plausibility check, but this is not really a check that the instance did not change.
With the tests as they are now we check in fact for a new instance.
In the free wild it is the duty of libraryResource to provide some resource as text
and it is the duty of readYaml to parse a text (... would also be possible to read
a file instead just parsing text, but that is a different story).
In our setup we just forwarded the resource name in libraryResource and reacted on
that resource name inside read yaml. There was no yaml parsing at all, the yaml 'as
it' was returned.
The approach now is much closer to reality. Library resource now provides the
text 'behind' the resource and yaml parses it. There is now a real yaml parsing.
Read yaml is now not registered explicitly anymore. It is just the readYaml closure
which is registerd by default for our tests.
read yaml rule is a very frequently used rule. But having the rule in the common rules
means we cannot register text or files to that rule, which makes it less handy to work
with yaml files in the tests.
