Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#9748 delete tools only added by tests #10274

Merged
merged 6 commits into from Feb 5, 2024
Merged

Conversation

sekmiller
Copy link
Contributor

What this PR does / why we need it: Updates ExternalToolsIT so that existing tools are not deleted

Which issue(s) this PR closes:

Closes #9748 Installed External tools deleted by running ExternalToolsIT

Special notes for your reviewer: Going in I thought that deleting the tools was a requirement to get the tests to pass. That turned out not to be the case which made the fix easier. The tests were modified to delete only those tools added via the tests.

Suggestions on how to test this: Install an external tool in your test environment. Run ExternalToolsIT
verify that your pre-installed tool(s) exist after running the test. And that there are no additional tools.
api for getting a list of installed tools:
curl http://localhost:8080/api/admin/externalTools

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?: no

Additional documentation: none

@coveralls
Copy link

coveralls commented Jan 26, 2024

Coverage Status

coverage: 20.136% (-0.004%) from 20.14%
when pulling bdc2c8e on 9748-fix-ExternalToolsIT
into 97508e6 on develop.

This comment has been minimized.

@@ -145,26 +130,14 @@ public void testFileLevelTool1() {
.statusCode(OK.getStatusCode())
// No tools for this file type.
.body("data", Matchers.hasSize(0));

//Delete the tool added by this test...
Response deleteExternalTool = UtilIT.deleteExternalTool(toolId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sekmiller for the other delete, you follow with an:
deleteExternalTool.then().assertThat()
.statusCode(OK.getStatusCode());
For consistency can you add that here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure

This comment has been minimized.

@pdurbin
Copy link
Member

pdurbin commented Jan 27, 2024

There's a test failure at https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10274/1/testReport/edu.harvard.iq.dataverse.api/ExternalToolsIT/testFileLevelTool1/

Seems like an easy fix:

1 expectation failed.
JSON path data[0].displayName doesn't match.
Expected: AwesomeTool
  Actual: Data Explorer

@sekmiller
Copy link
Contributor Author

Weird. It's not failing for me locally.

@cmbz cmbz added the Size: 3 A percentage of a sprint. 2.1 hours. label Jan 30, 2024

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:9748-fix-ExternalToolsIT
ghcr.io/gdcc/configbaker:9748-fix-ExternalToolsIT

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@sekmiller sekmiller assigned pdurbin and unassigned sekmiller Jan 31, 2024
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are passing. Code looks good. Will merge.

@pdurbin pdurbin merged commit 98231c5 into develop Feb 5, 2024
18 checks passed
@pdurbin pdurbin deleted the 9748-fix-ExternalToolsIT branch February 5, 2024 21:14
@pdurbin pdurbin added this to the 6.2 milestone Feb 5, 2024
@pdurbin pdurbin removed their assignment Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Installed External Tools deleted by running Integration Test
5 participants