Fix to enforce oneOf branch as per the schema

[davrodgon]

The method allows for code execution paths that are not valid according to the schema, namely executing both a commands and a tox block. This is forbidden in the schema by the following clause. "oneOf": [ { "required": ["commands"] }, { "required": ["tox"] } ],

This will not happen as long as the validator is executed and works properly. However, in my opinion this code itself should not allow the potential execution of a forbidden execution path, in particular when it is very easy to avoid it. If not fixed it would be possible for a future bug that affects the call to the validator or the validator itself to enable such forbidden execution paths, causing unexpected behaviours.

[orviz]

Hi @davrodgon not sure what is your proposal, don't see any changes in the PR

[davrodgon]

Hi @davrodgon not sure what is your proposal, don't see any changes in the PR

Sorry, I had moved to the master branch to bring it up to date with the one in indigo, and I didn't change back to the local fix branch before pushing. Anyway, it is just adding the "else" to avoid the illegal branch and reduce the possible paths to 3. The third one would be the one that executes a predetermined code for some criteria - e.g., QC.Acc, QC.Lic.

[orviz]

ok now I see it. True that there is sort of contradiction among the validator and the code. It looks good to me this change, do you agree @samuelbernardolip ?

[samuelbernardolip]

ok now I see it. True that there is sort of contradiction among the validator and the code. It looks good to me this change, do you agree @samuelbernardolip ?

This PR was based on wrong base branch as I could understand. I changed the base branch into release/2.1.0 and it adds additional changes to workflow... @davrodgon can you review this detail?

Looking from JSON schema perspective, this is a good point from @davrodgon, but I need to review this in more detail, since with the new architecture those if conditions will be removed. But from Composer implementation I don't see any reason for not supporting both commands, tox or any other build tool in same stage. Lets look in more detail for an example:

sqa_criteria:
  qc_doc:
    repos:
      repository_name:
        container: image_name
        commands:
          - script1
          - script2
        tox:
          tox_file: /repository_name/tox.ini
          testenv:
            - stylecheck      

In this example the stage name is "qc_doc repository_name" and its steps are script1, script2 and tox stylecheck by this order.

In my opinion we should correct the schema to allow this since there is no reason to limit each stage to a specific executor (build tools or commands).

[orviz]

After some discussion, we realize that there should be no inconvenient in allowing multiple builder definitions in the same stage. Accordingly, we modified the jpl-validator to set anyOf instead (check https://github.com/EOSC-synergy/jpl-validator/pull/36). With this change, the current PR can be closed (unmerged)