Skip to content

Conversation

@dlmarion
Copy link
Contributor

@dlmarion dlmarion commented Oct 5, 2023

No description provided.

@dlmarion dlmarion requested a review from keith-turner October 5, 2023 15:13
@dlmarion dlmarion self-assigned this Oct 5, 2023
@dlmarion
Copy link
Contributor Author

dlmarion commented Oct 5, 2023

FYI that I wanted to add the Sunny tag to ServiceLockIT as well, but I got some error about the Tag that didn't make sense to me.

@keith-turner
Copy link
Contributor

Adding the test for bulk, merge, compaction, etc gives good high coverage of the major features of Accumulo. The following test seem to exercise internal impl details of the high level features that would be partially exercised by the test that exercise the high level features.

  • LocatorIT
  • ManagerAssignmentIT
  • TabletManagementIteratorIT
  • TabletManagementScannerIT
  • TabletResourceGroupBalanceIT

So thinking maybe these test should not be added.

@dlmarion dlmarion merged commit bdac4cd into apache:elasticity Oct 5, 2023
@dlmarion dlmarion deleted the add-more-tests-to-sunny branch October 5, 2023 21:09
Copy link
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

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

Some of these tests seem like more than just quick smoke tests to make sure we haven't fundamentally broken some basic core functionality. I would not consider merges or all of the edge case scan server ITs to be basic core functionality, maybe not even bulk import. Yes, they are important, but they can be run when the full ITs run... or we can create a new category if needed. But, the purpose of the sunny tests are to quickly run checks, and adding a bunch more to them is going to make smoke testing contributions harder.

I would suggest rolling these out, and creating a new category for elasticity-related features, if there is a need for such a category. Maybe one or two of these could be added to sunny, but I don't think all of them should be.

@dlmarion
Copy link
Contributor Author

dlmarion commented Oct 6, 2023

@ctubbsii - @keith-turner and I talked about this and I think we both agreed that for now, for the elasticity branch, we are good with marking these as part of the sunny tests. These tests are being built periodically by a Jenkins server, it's not something that we are doing manually. All of the tests are not working yet on the elasticity branch and this gives us a good indication that the major features are working. We can revisit this when all of the ITs are working.

@keith-turner
Copy link
Contributor

We also discussed #3826 that could be created in 2.1 or 3.1 and merged forward into elasticity. In elasticity this new test could replace alot of the test that were tagged for sunny in this PR. If #3826 is not done, then we #3825 to remind us to reevaluate what is tagged as sunny.

keith-turner added a commit to keith-turner/accumulo that referenced this pull request Oct 23, 2023
@ctubbsii ctubbsii added this to the 4.0.0 milestone Jul 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants