Issue Details (XML | Word | Printable)

Key: NET-198
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Unassigned
Reporter: Sebb
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
Commons Net

FTPTimestampParserImpl#parseTimeStamp() is not fully testable

Created: 08/Mar/08 10:09 AM   Updated: 09/Mar/08 10:58 PM
Return to search
Component/s: None
Affects Version/s: None
Fix Version/s: 2.0, 1.5

Time Tracking:
Not Specified

File Attachments:
  Size
Text File Licensed for inclusion in ASF works FTPTimestampParserImpl.patch 2008-03-08 10:12 AM Sebb 0.9 kB
Text File Licensed for inclusion in ASF works FTPTimestampParserImpl2.patch 2008-03-09 04:55 PM Sebb 1 kB
Issue Links:
Blocker
 

Resolution Date: 09/Mar/08 05:35 PM


 Description  « Hide
The FTPTimestampParserImpl#parseTimeStamp() method is not fully testable, because it unconditionally creates Calendar items using the current time.

In order to test for leap years and DST, the test code needs to be able to set arbitrary times.

I suggest adding a package-private method that takes an additional Calendar parameter, as follows:

Calendar parseTimestamp(String timestampStr, Calendar now) throws ParseException {
// etc

This would replace the original code; the public interface would delegate to the package-private method:

public Calendar parseTimestamp(String timestampStr) throws ParseException { Calendar now = Calendar.getInstance(); return parseTimestamp(timestampStr, now); }

Patch to follow.



 All   Comments   Work Log   Change History   Subversion Commits      Sort Order: Ascending order - Click to sort in descending order
Sebb added a comment - 08/Mar/08 10:14 AM
It's not possible to test fixes to NET-188 without this patch

Sebb added a comment - 09/Mar/08 04:52 PM
Patch needs adjusting

Sebb added a comment - 09/Mar/08 04:55 PM
The original patch was changed slightly when it was applied, and one of the lines was moved to the wrong method.

Also, it's important that the fix to the year is taken from the provided year, not the current year (which may be different in testing).
This was omitted in the original patch.