Opened 2 years ago

Closed 2 years ago

#22280 closed enhancement (implemented)

check tests and parsing for duplicate keys and empty keys exceptions

Reported by: iwakeh Owned by: iwakeh
Priority: Medium Milestone: metrics-lib 1.9.0
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

This was discovered in #22217 in ExtraInfoDescriptorTest's padding-count tests (comments 6,7).
Check other tests in ExtraInfoDescriptorTest and other test classes for similar issues and fix these

Child Tickets

Change History (10)

comment:1 Changed 2 years ago by iwakeh

Milestone: metrics-lib 1.8.0

Good candidate for 1.8.0.

comment:2 Changed 2 years ago by iwakeh

Owner: changed from metrics-team to iwakeh
Status: newaccepted

comment:3 Changed 2 years ago by karsten

One day until the planned 1.8.0 release. Should we move this to 1.9.0, or is this already almost done?

comment:4 in reply to:  3 ; Changed 2 years ago by iwakeh

Replying to karsten:

One day until the planned 1.8.0 release. Should we move this to 1.9.0, or is this already almost done?

This should go into 1.8.0; update following.

comment:5 in reply to:  4 Changed 2 years ago by iwakeh

Replying to iwakeh:

Replying to karsten:

One day until the planned 1.8.0 release. Should we move this to 1.9.0, or is this already almost done?

> This should go into 1.8.0; update following.

OK, I checked and there are way too many expected-exception-tests to be changed for this release.
It is important to have these. I'll check-in some changes later, but these might not need to go in now.
So I'd suggest too to move this ticket to 1.9.0

comment:6 Changed 2 years ago by karsten

Milestone: metrics-lib 1.8.0metrics-lib 1.9.0

Sounds good. Moving to 1.9.0.

comment:7 Changed 2 years ago by iwakeh

First batch of checking messages in expected exceptions is here.

There are 270 left:

src/test/java/org/torproject/descriptor/impl/RelayNetworkStatusConsensusImplTest.java:100
src/test/java/org/torproject/descriptor/impl/RelayNetworkStatusVoteImplTest.java:90
src/test/java/org/torproject/descriptor/impl/ServerDescriptorImplTest.java:80

And, of course the empty and duplicate key changes to tests.

comment:8 Changed 2 years ago by iwakeh

Finally all exceptions in tests are verified (i.e., the exception's message). For all the other changes it seems that the introduction of "KeyValueMap" already completed the simplification of ParseHelper and duplication/empty key checking in #22279. The class 'KeyValueMap' has full coverage. Thus, this tasks seems to be finished.
Please review this branch.

comment:9 Changed 2 years ago by iwakeh

Status: acceptedneeds_review

comment:10 Changed 2 years ago by karsten

Resolution: implemented
Status: needs_reviewclosed

Great, thanks for going through all that test code! Squashed, rebased to master, added another commit for replacing remaining @Test() annotations with @Test, and pushed to master. Closing.

Note: See TracTickets for help on using tickets.