I1 #894
I1 #894
Conversation
Instead of parsing the CICS.xml. Copy the bundle directory from the Galasa resources directory, copying the bundle parts according to the part type in either binary or text, after applying substitutions.
|
Automatic triggering of the build is cancelled as user is not a member of the approved groups. Please ask an admin to review this PR and create a comment on the PR stating 'Approved for building' |
|
Hi @galasa-team I need a review on this change. I can't assign anyone. |
| logger.debug("Creating CICS Bundle Directory"); | ||
| try { | ||
| this.runTemporaryUNIXPath = this.zosFileHandler.newUNIXFile(cicsRegion.getRunTemporaryUNIXDirectory().getUnixPath() + "CICSBundles" + SLASH_SYBMOL + getName() + SLASH_SYBMOL, this.cicsZosImage); | ||
| this.runTemporaryUNIXPath = this.zosFileHandler.newUNIXFile(cicsRegion.getRunTemporaryUNIXDirectory().getUnixPath() + "CICSBundles" + SLASH_SYBMOL , this.cicsZosImage); |
There was a problem hiding this comment.
SYMBOL is spelt badily ;-)
|
|
||
|
|
||
|
|
||
| public void copyBundleFilesToZfs() throws ZosUNIXFileException, IOException { |
There was a problem hiding this comment.
Pretty complex logic in this method. Any plans to add some unit tests to make sure it does what you expect ?
| throw new CicsJvmserverResourceException(RESOURCE_TYPE_BUNDLE + " " + getDefinitionName() + " does not exist"); | ||
| } | ||
| this.cicsRegion.cemt().setResource(this.cicsTerminal, RESOURCE_TYPE_BUNDLE, getName(), "DISABLED"); | ||
| this.cicsRegion.cemt().setResource(this.cicsTerminal, RESOURCE_TYPE_BUNDLE, getDefinitionName(), "DISABLED"); |
There was a problem hiding this comment.
Not sure what it does, but pls pardon the obvious question: Should "ENABLED" and "DISABLED" be symbolic constants rather than literals, so they can be re-used elsewhere and documented what values are allowed ? If it's only those two possible values, should it be a boolean instead of a string ?
There was a problem hiding this comment.
Not got to try it as there's no jvmserver support for my test bundles. Possibly could be, and maybe more states.
| this.cicsRegion.cemt().setResource(this.cicsTerminal, RESOURCE_TYPE_BUNDLE, getDefinitionName(), "DISABLED"); | ||
| } catch (CicstsManagerException e) { | ||
| throw new CicsBundleResourceException("Problem disabling " + RESOURCE_TYPE_BUNDLE + " " + getName(), e); | ||
| throw new CicsBundleResourceException("Problem disabling " + RESOURCE_TYPE_BUNDLE + " " + getDefinitionName(), e); |
There was a problem hiding this comment.
Is it ever possible that getDefinitionName returns null ? In which case would this cause a NullPtrException ? Better to use one of the formatting APIs like MessageFormat.format(template,parameter...) which copes with nulls as substitution strings ?
There was a problem hiding this comment.
Not got to try it as there's no jvmserver support for my test bundles.
| * @return the CICS BUNDLE name | ||
| */ | ||
| public String getName(); | ||
| public String getDefinitionName(); |
There was a problem hiding this comment.
Normally, you'd deprecate the public interface method, rather than renaming/removing it.
How sure are we that nobody is going to break as a result of this change ?
Could any of the Galasa users out there be using this interface ?
|
Approved for building |
|
What is needed for this to merge ? |
3 participants
No description provided.