Use knora-api 13.0.0-rc.5

[tobiasschweizer]

Use knora-api 13.0.0-rc.5

[tobiasschweizer]

The unit tests fail because of a change in v2/values/create-value-response.json:

{
  "@id" : "http://rdfh.ch/0001/_GlNQXdYRTyQPhpdh76U1w/values/OGbYaSgNSUCKQtmn9suXlw",
  "@type" : "knora-api:IntValue",
  "knora-api:valueCreationDate" : {
    "@type" : "http://www.w3.org/2001/XMLSchema#dateTimeStamp",
    "@value" : "2019-01-09T15:45:54.502951Z"
  },
  "knora-api:valueHasUUID" : "hKOvV-6ZSG-qnOTKHRmlfQ",
  "@context" : {
    "knora-api" : "http://api.knora.org/ontology/knora-api/v2#"
  }
}

The response now contains a knora-api:valueCreationDate.

update-value-response.json did not change.

@benjamingeer Did the value creation response always contain a knora-api:valueCreationDate or is this new in v13.0.0-rc.5?

[benjamingeer]

Did the value creation response always contain a knora-api:valueCreationDate or is this new in v13.0.0-rc.5?

This was added recently, in dasch-swiss/dsp-api#1646.

I thought we had all agreed that knora-api-js-lib shouldn't fail if Knora returned additional information, so this shouldn't cause a test to fail.

[tobiasschweizer]

I thought we had all agreed that knora-api-js-lib shouldn't fail if Knora returned additional information, so this shouldn't cause a test to fail.

knora-api-js-lib does not fail. The thing that failed was an assertion in the specs that dynamically creates the response data for the different value types. The assertion compares the dynamically created data with the JSON obtained from Knora.

[tobiasschweizer]

and none of the E2E tests failed either

[benjamingeer]

The assertion compares the dynamically created data with the JSON obtained from Knora.

OK but that means that if Knora returns additional information, the test considers it a failure (a breaking change), which it isn't. So maybe that test isn't a good idea.

[tobiasschweizer]

The assertion compares the dynamically created data with the JSON obtained from Knora.

OK but that means that if Knora returns additional information, the test considers it a failure (a breaking change), which it isn't. So maybe that test isn't a good idea.

Without the assertion there would be no way to know whether the response is correct. Knora returns a value creation response for an integer value. But all the other types have to be tested too. These can be easily produced by the test framework, but there needs to be a check if it actually conforms to what Knora returns.

If you provide test response data for each value type, then this can be removed.

[benjamingeer]

Without the assertion there would be no way to know whether the response is correct.

Why not just check for the presence of the data that you're looking for, and ignore the rest?

Why is the value type in the response relevant? Does knora-api-js-lib use it for anything?

[tobiasschweizer]

Why is the value type in the response relevant? Does knora-api-js-lib use it for anything?

It's actually not relevant. My rationale was that the tests should be realistic and numb. I admit that adding the mocks to produce the data dynamically introduces some additional complexity that could also lead to problems. As always when you actually test the test.

I will create a DSP task to evaluate taking a simpler approach.

However, shouldn't all changes in Knora's public API be documented in the GitHub releases? Also a non breaking change is of interest. Or did I look in the wrong place?

[tobiasschweizer]

created a task here: https://dasch.myjetbrains.com/youtrack/issue/DSP-418

[benjamingeer]

However, shouldn't all changes in Knora's public API be documented in the GitHub releases?

True, I've added it here:

https://github.com/dasch-swiss/knora-api/releases/tag/v13.0.0-rc.5

[tobiasschweizer]

However, shouldn't all changes in Knora's public API be documented in the GitHub releases?

True, I've added it here:

https://github.com/dasch-swiss/knora-api/releases/tag/v13.0.0-rc.5

Thank you!

[tobiasschweizer]

just a detail: as a convention, in JS-lib and UI-lib we link to the PRs. See https://github.com/dasch-swiss/knora-api-js-lib/releases for example. I also think it is helpful to have different categories.

Maybe we could make this consistent throughout DSP?

[benjamingeer]

in JS-lib and UI-lib we link to the PRs

Good idea.

[kilchenmann]

For the release notes we use GREN in github actions (CI)

[kilchenmann]

I can add our "Generate release notes" process to the DSP-API repo, if it's ok?

[benjamingeer]

I can add our "Generate release notes" process to the DSP-API repo, if it's ok?

Is that a program? I think we already have GREN, but we forgot to document it, so I've forgotten how to use it.

[benjamingeer]

@kilchenmann This is what we have so far:

https://github.com/dasch-swiss/knora-api/blob/develop/RELEASING.md

[kilchenmann]

Yes, it's also GREN. We have integrated the command in Github CI. So, it will update the release notes automatically on each release.
Or if you like to do it manually, we should update the grenrc configuration to use the same template (with the links) as we use in dsp-js-lib, in dsp-ui-lib and in dsp-app.

[benjamingeer]

Let's try integrating it in GitHub CI like you did. If we need to add something to the generated release notes, we can always edit them manually after they're generated.

[kilchenmann]

Not sure if we already should merge or if we can wait for missing test data? S. DSP-423 → the requested test data (responses) exists already but not as post response data, because it's the same as the get request data. s. Ben's comment

[tobiasschweizer]

Not sure if we already should merge or if we can wait for missing test data? S. DSP-423

Would this PR be useful for you?

[benjamingeer]

Not sure if we already should merge or if we can wait for missing test data? S. DSP-423

Doing that now.

[kilchenmann]

LGTM

[kilchenmann]

We should merge this PR asap. I need the create ontology response in PR #208

[tobiasschweizer]

@kilchenmann Ok, it has been merged.

[kilchenmann]

@tobiasschweizer Thanks. I will update my branch...