#6920 Informative error message for custom metadata block definition.#6921
Conversation
|
@pkiraly I'm not able to run the testLoadMetadataBlock_ErrorHandling in AdminIt because I don't have a copy of the test.tsv file. Can you add it to the project? thanks |
|
@sekmiller Sorry, I forgot to add it. Now it is there. |
sekmiller
left a comment
There was a problem hiding this comment.
Looks good overall. Your test is failing - see comment below. I'd suggest putting this new error message in the bundle.
|
|
||
| String message = JsonPath.from(body).getString("message"); | ||
| assertEquals( | ||
| "Error parsing metadata block in DATASETFIELD part, line #5: missing 'watermark' cell (#5)", |
There was a problem hiding this comment.
This test is failing because the test has "cell" in the error message while the api uses "column". Suggest putting the error message into the Bundle. None of the rest of the endpoints use the bundle because this is a very old api, but we're trying to add new work to the bundle when possible
There was a problem hiding this comment.
Oh, and you'll need to get the latest from the develop branch.
|
@sekmiller I added bundles, and fixed the cell vs column issue. I also added an extra unit test class for the API class, starting with a simple thing: testing the bundle outputs, and commited these changes, However after that I tried to update with the latest development changes, but I guess I issued wrong git commands (I was in the 6920-informative-error-message-for-custom-metadata-block-definition branch in my local forked dataverse repo): So now in the changed files tab I see 12 files changed, while I changed only 6 files, so it contains several extra commits. Do you have idea how to remove them while keeping the branch sync with the devel branch? |
d899916 to
8c331df
Compare
|
@sekmiller I removed the wrong commit. Now it shows no conflicts with the base branch |
…GeneralErrorMessage().
|
@sekmiller It is strange. This short report (in this page) says: "continuous-integration/jenkins/pr-merge — This commit has test failures", however when I click on the details, I see a timeline with three important stages: build, test, and post. The build and test phases are in greens (denoting that they went well), while the post phase is orange. It gives only a simple message: "Recording test results" which doesn't sound as an error. |
|
@pkiraly @sekmiller there are some transient test failures in the integration test suite https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/view/change-requests/job/PR-6921/lastCompletedBuild/testReport/ File replace tests are some of the most common. The testAdditionalDatasetContent6300 might be worth looking into; I don't remember seeing it before. |
|
@donsizemore @sekmiller I was not able to reproduce the testAdditionalDatasetContent6300 issue in the local environment. From the Jenkins's log I get the following: I am not a PSQL expert, but it seems it might be a multithreading or transactional problem. The other test failure is testForceReplaceAndUpdate() in FilesIT class. It expects HTTP status code 200 but gets 400, which mean Bad Request, Which calls I tried to debug it with the following cURL command (I am not sure if I transformed the API into the perfect cURL parameters): it returns HTTP 400 and a similar error message: I have checked, and I haven't seen sign in FilesIT which set a clean state of the database before every tests, so I guess this test fails because the same information is already created by a test. However all of these seems quite independent from the current custom metadata block definition API calls. |
|
This PR was developed by Göttingen eResearch Alliance, Germany, and funded by SSHOC, "Social Sciences and Humanities Open Cloud". SSHOC has received funding from the European Union’s Horizon 2020 project call H2020-INFRAEOSC-04-2018, grant agreement #823782. |
What this PR does / why we need it: Improve the error handling of the custom metadata block definition API call
Which issue(s) this PR closes: #6920
Closes #6920
Special notes for your reviewer:
Suggestions on how to test this:
I have created an integration test for it. If you would like to run only that test, run the following:
Does this PR introduce a user interface change?: no
Is there a release notes update needed for this change?: no
Additional documentation: no