#48 Remove system from productmd-compose.test.error example
Merged 4 years ago by mvadkert. Opened 5 years ago by jbieren.
fedora-ci/ jbieren/messages test-error-fix  into  master

@@ -12,13 +12,6 @@ 

      "log_raw": "https://rtt.somewhere.com/job/compose-RHEL-ALT-7-nightly-tier2-acceptance/arch=aarch64,variant=Server/2/consoleText",

      "log_stream": "https://rtt.somewhere.com/job/compose-RHEL-ALT-7-nightly-tier2-acceptance/arch=aarch64,variant=Server/2/consoleText"

    },

-   "system": [

-     {

-       "variant": "Server",

-       "architecture": "aarch64",

-       "provider": "beaker"

-     }

-   ],

    "artifact": {

      "type": "productmd-compose",

      "id": "RHEL-ALT-7-20180102.n.0",

@@ -22,10 +22,6 @@ 

          $ref: error.json

      notification:

          $ref: notification.json

-     system:

-         type: array

-         items:

-             $ref: productmd-compose-system.json

      image:

          $ref: productmd-compose-image.json

      generated_at:
@@ -39,7 +35,6 @@ 

      - artifact

      - pipeline

      - test

-     - system

      - error

      - generated_at

      - version

productmd-compose.test.error is the only *.test.error with system in the message. I believe it should be removed.

Signed-off-by: Johnny Bieren jbieren@redhat.com

yep, for error messages it makes little sense to require it ... as you can get in a situation that you are not even able to provision ...

Well, there's problem with this. We run our tests for each variant and architecture separately, so we send multiple messages for the compose using system to to identify which variant/architecture is being tested. If one of those crashes but other end successfully, we need to identify which one crashed, but without the system, we lose the information.

Based on what @pholica reports here, it seems like system should be added to the other *.test.error messages instead of removed here.

Do you agree @mvadkert, @jbieren?

@ralph that is fine by me. I just like consistency. And if it makes sense for one of them, it should make at least a little sense in the other similar ones, so sure +1 to adding it to all

Yeah, I guess that makes sense. Even though it can be just an optional field :( as this information on the other hand does not need to be present. But I believe system is optional anyway.

@mvadkert anything can be made optional. The more things that are optional, the less things any listener can rely on being there and the less that can be imposed by a shared library -> the less useful a shared library is. Every single field could be made optional, making a shared library useless and the messages a total free-for-all

Well, I've started thinking if we want to run the architectures and variants as separate test runs or aggregate results of the variants/architectures in test case results so we may not need to distinguish which ended with error since this is what the pipeline is for, right?

@pholica if you have results per that combination, I guess architecture information is provided in some other fields, so yeah, I guess you do not need it :)

So agreed to get it dropped?

@pholica remove the WIP keyword pls if you are ready :)

I think @pholica is fine afterall, @pholica also note that if the system is not present in the spec, you can still use it :)

Pull-Request has been merged by mvadkert

4 years ago