Fix DataRetrieverApiIT.testRetrieveMyDataCollections() for github mig…#12372
Fix DataRetrieverApiIT.testRetrieveMyDataCollections() for github mig…#12372stevenwinship wants to merge 2 commits intodevelopfrom
Conversation
|
related to issue #12368 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
After a quick check a few days back when I changed User 1 to greater than or equal to, the assertions seemed to be failing for User 2 and User 3 as well. There was always one extra dataset than expected. It was a different ID from user1's extra collection but it remained consistent from test to test. This is the primary reason why I changed it to disabled instead. |
3a68a2f to
1ecd12c
Compare
|
@srmanda-cs Did the new test show that the root count changed from 1 to 2? If not then there is another issue. The root count should be added to each user's count and therefore work. I can't get it to fail locally so it's hard to test |
|
📦 Pushed preview images as 🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
I realize that it is incredibly difficult to test, because it is so environment specific :( I have little to no idea why that specific test is failing. All I realize is that when I changed it to greater than or equal to for user1, user2 returned 18 count instead of the expected 17. The root dataverse count remains constant, it is the number of other dataverses that the users have access to that have increased. Its very similar to how the Mail test was the only one failing along with this when I just ran a full Maven verify. And as I have mentioned to Phil previously, I really don't want to create more work for y'all than you already have. I can provide you with more testing, more logs, and more specific details but it will take me a lot of time to dig into this as its a novel codebase to me. Which is why I sought your help. |
|
I'm thinking we should merge the new GitHub Action first: Then we should rebase this PR (#12372) after the merge and see if it's passing or not. |
|
@srmanda-cs It's not making more work for us. It's helping us understand the issues that can arise from the migration. I have gone through this migration before between Jenkins and GitLab. Asynchronous tests and Test suites that have different settings (JVM, etc.) can greatly speed up testing but do make testing a bit more challenging. |
|
This is one of the runs that outlines the specific test failure: https://github.com/IQSS-Dataverse-Development/dataverse/actions/runs/25069771575 The specific workflow file, the code changes, the full docker logs, and the full execution run can be obtained from these single links. If you click on the workflow button, then go to the gear icon, and click download raw logs you should see multiple megabytes worth of raw logs. This is what I've noticed happens when I change it to greater than or equal to for user1, user 2 starts failing: https://github.com/IQSS-Dataverse-Development/dataverse/actions/runs/25071885059 I believe these logs to be very comprehensive and thorough, but also immensely dense and hard to read. I genuinely appreciate your help! My initial thoughts were that they're a permissions issue letting :authenticated-users see more dataverses than they're allowed to. But jenkins truncates all test output, so I couldn't directly verify it. Please let me know wherever I can help, I am ready to jump in. |
|
I think the better solution is to not look at the size of the list but to verify the contents include the desired collections |
If it fits the purpose of why the test exists, then it sounds like an amazing solution to me! |
What this PR does / why we need it: DataRetrieverApiIT.testRetrieveMyDataCollections() fails in Github
Which issue(s) this PR closes: #12371
Special notes for your reviewer:
Suggestions on how to test this:
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: