Introduce FrameworkType and set Run name of Resource Management

[adamcoulthard]

fixes galasa-dev/projectmanagement#2124

This is the first part of the Resource Management changes. To allow RAS directory pieces to start to work correctly this introduces the new internal FrameworkType enum at the moment that are the following enums:

• test
• resourceManagement
• ecosystem
• launcher
• controller
• server

Some of these could be expanded but at the moment the only two that actually mean anything are test and resourceManager. When the framework is initialised it uses these to decide what to do with the testRunName which is either set to the next run number if a test or "resourceManagement" if its the resourceManagement, otherwise no RunName is set.

[techcobweb]

So far in Galasa development we've been really trying hard to not make the framework know how it's being used.
Not looked at the code yet, but this is something we've been actively trying to avoid.
Any variance we've added a plug-point, and the different configurations inject a different implementation.
eg: Ras using etcd or Ras using a file.
Will look at the code and get back to you...

...some time passes...

OK. Looked at the code. It's clear we didn't follow our own advice on that one (soo inconsistent), so what you are doing isn't a huge stretch beyond what's already there.

And this PR isn't disruptive I believe.

[techcobweb]

This "resourceManagement" should be a symbolic literal from somewhere.

[adamcoulthard]

Any ideas where might be best to place this? Is there a standard location for these things? For the time being I could just add it to the FrameworkInitialisation.java.

[techcobweb]

Might be worth putting this into a separate method so the overall method doesn't get any bigger.

[techcobweb]

TEST, RESOURCE_MANAGEMENT, ECOSYSTEM, LAUNCHER, CONTROLLER, SERVER - I always thought enums should be upper-case names. I see there are differing opinions online about this, but generally in Java I think all constants are upper-case, and enum values are constants.

What do each mean ? eg: What is an ECOSYSTEM type of framework ? Not sure that means anything to me.

Also, trying to move away from the word "ecosystem" in general anyway

What's the difference between a LAUNCHER and SERVER ?...etc.

As you don't have a need for some of these types, maybe have TEST, RESOURCE_MANAGEMENT, OTHER ? until we do need the distinctions ?

[adamcoulthard]

I did wonder about not doing them all, happy to change to these.

[techcobweb]

I see what you're doing. The changes don't look disruptive. Some minor points raised in the comments.

[techcobweb]

In some ways I wonder whether we should just pass in a TestNameAllocator(), and OverridePropertiesLoader() interface/class rather than do this, to drive the differences in behaviour. That would make for fewer branches in the code, and fewer unit tests to write generally.

Or (better still) have a FrameworkTypeStrategy interface, which gets passed in, which hides the differences between the different 'modes' of running.
eg:

public interface FrameworkInitialisationStrategy {

  /** @returns null if no testcase name needs to be set
  String getTestcaseName(cps);

  void startLoggingCapture(framework);

  Properties loadOverridePropertiesFromDss(dss);
 
  ...etc.
}

We could just have a Test implementation, and a ResourceManager implementation, and a default implementation.

We could also extract some code out of the initialiser class which is specific to Test initialisation, or resource manager initialisation..etc. eg: locateRunName()

Is it important for your future plans that the type lives beyond the initialisation of the framework, to provide different behaviour? Like what ?...

[techcobweb]

Also, unit testing... any scope for some unit tests at all here ?

If unit testing can be done without relying on any framework, just straight Java old-school way, then we lessen our dependencies also.

The unit testing gets easier if we inject a strategy, and unit test that each implementation of the strategy does what it's supposed to independently of the base framework code.

[techcobweb]

@adamcoulthard Thanks for the PR to look at. Comments above. Happy to talk about anything I've written there.

[adamcoulthard]

@techcobweb there is no reason for anything to live on. The only reason really for this change is to allow me to set the testName for the resourceManager and prior to these changes I couldn't do it. This allows the managers to be able to initialise their Ras Providers correctly.

I think longer term we probably want to look at the managers and how the RAS stuff works so that we can make a null RAS provider that just "swallows" everything. But I've not really understood full how all the RAS stuff hangs together and was a lot more nervous about making changes in that area than this simple fix now. With potential enhancing in the future.

I would probably agree that doing something slight different to all this would make sense but I was worried about over complicating the startup. Its already difficult to weave though and I thought more indirection would make things worse.