[jira] [Commented] (CASSANDRA-17009) Sstableverify unit test operate on SSTables

["Berenguer Blasi (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-17009?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17423135#comment-17423135 ] 

Berenguer Blasi commented on CASSANDRA-17009:
---------------------------------------------

> I accidentally turned off the spell checker in the ide as it turns out... blerg I'll fix them, sorry about that.

No need to apologize! I do these sort of things all the time unfortunately lol!

> The fix for this is small, but I was trying to keep the change small, incremental and focused on the unit test.  No sweat though, I can fix it (basically just force an early exit).

And you're absolutely right. But from experience I can tell you each ticket is sandwiched between CI and contributor review cycle time. That tends to be not negligible so if it's a few loc of a related fix that'd be my suggestion to keep things moving. But feel free to do whatever you think is best ofc!

> The issue with the git ignore of test/data sounds weird to me it didn't came up sooner. I have to give it another thought.

Ok I'll buy that.

> On the {{Verifier}} and coverage discussion

Icwym and I think you are right. Still I would suggest:
- Adding to all verify test classes a javadoc detailing all the files involved: VerifyTest, StandaloneVerifierOnSSTablesTest, etc This way a reader will learn immediately about coverage and what classes are involved instead of wondering around and finding it out himself. Wdyt?
- Yes replicating those tests would be redundant and we already have existing tests on the cmd line options making sure the tool doesn't explode but
- The -c test we need to add to {{StandaloneVerifierTest}} bc we have no test proving this option doesn't explode the tool or that the tool is ignoring it i.e. Same as with the token option which in this case is another ticket. But -c, being minimal effort, I'd suggest doing in this ticket.

Wdyt makes sense? I'll move the state of the ticket to help me track things if you don't mind.

> Sstableverify unit test operate on SSTables
> -------------------------------------------
>
>                 Key: CASSANDRA-17009
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-17009
>             Project: Cassandra
>          Issue Type: Sub-task
>          Components: Tool/sstable
>            Reporter: Brian Houser
>            Assignee: Brian Houser
>            Priority: Normal
>          Time Spent: 2h
>  Remaining Estimate: 0h
>
> as part of https://issues.apache.org/jira/browse/CASSANDRA-16009, unit coverage is a bit lax and doesn't run through the verifier (based on my coverage results).
> There should be a unit test that exercises the internal verifier both for a corrupt example and a working example.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org