Edgewall Software

Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#11118 closed enhancement (fixed)

Add a functional test case for milestone attachments

Reported by: Ryan J Ollos <ryan.j.ollos@…> Owned by: Ryan J Ollos
Priority: normal Milestone: 1.0.2
Component: attachment Version: 1.0-stable
Severity: normal Keywords: milestone attachment functional test case
Cc: Branch:
Release Notes:

Added a functional test case for milestone attachments.

API Changes:

Added method attach_file_to_milestone to the FunctionalTester class.

Internal Changes:


I've recently experienced an issue with a theme failing to provide attachment functionality for milestones, therefore I propose to add a functional test case (or multiple test cases) for milestones attachments.

Attachments (0)

Change History (9)

comment:1 by Ryan J Ollos <ryan.j.ollos@…>, 9 years ago

Keywords: milestone attachment functional test case added

Now that I've opened two tickets for adding functional test cases (#11109 being the other), I just wanted to ask if I should continue to open a ticket per significant test case that is needed, or if there is some other way that you'd like me to approach this. I just want to make sure that I'm not generating too much noise in the ticket system

comment:2 by Remy Blank, 9 years ago

Your current approach is fine.

comment:3 by Ryan J Ollos <ryan.j.ollos@…>, 9 years ago

Here is a stack of changes that accomplishes my goals for this ticket and cleans up a few other things:

  1. cf4d963b: (Unrelated) Removed unused code in batch.py.
  2. cbfc1f2c:Added a go_to_milestone method to the FunctionalTester class.
  3. 140de6abAdded a attach_file_to_milestone method to the FunctionalTester class.
  4. b928492f: Added a test for milestone attachments.
  5. b5ee4722: Refactored common code in FunctionalTester into _attach_file_to_resource.
  6. 871c64d1: Refactored FunctionalTester._attach_file_to_resource (see detailed changes in commit message).
  7. d113b5ca: Renamed tempfilename to filename for conciseness and clarity.
  8. b6789076: Replaced == and != with is and is not when rhs is None.

I learned something rather unfortunate while implementing this. The regex for (4) has the rel attribute before the href attribute, which was required for the test to pass. However, when inspecting the HTML in Firefox for this failing test, the rel attribute came after the href attribute. I'm not surprised that this is browser dependent, but I suppose it would be desirable to call twill's find in a way that would not depend on the order of the attributes.

comment:4 by Ryan J Ollos, 8 years ago

Component: generalattachment
Milestone: 1.0.2
Owner: set to Ryan J Ollos
Status: newassigned
Version: 1.0-stable

comment:5 by Ryan J Ollos, 8 years ago

API Changes: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Refactorings committed in [11894:11898] and merged to trunk in [11900].

Functional test case added in [11899] and merged to trunk in [11901].

The ==is refactorings in [11895] should have been part of a different changeset, but I guess I didn't commit one of the patches before applying the next.

comment:6 by Jun Omae, 8 years ago

Resolution: fixed
Status: closedreopened

1 test failure on trunk. It seems that [11831] isn't merged into trunk and is marked "merged" on svn:mergeinfo in [11853].

Add attachments to the milestone. ... ERROR

ERROR: Add attachments to the milestone.
Traceback (most recent call last):
  File "trac/ticket/tests/functional.py", line 1065, in runTest
    filename = self._tester.attach_file_to_milestone(milestone_name)
  File "/home/jun66j5/src/trac/edgewall/github/trac/tests/functional/tester.py", line 313, in attach_file_to_milestone
AttributeError: 'FunctionalTester' object has no attribute 'go_to_milestone'
Last edited 8 years ago by Jun Omae (previous) (diff)

comment:7 by Ryan J Ollos, 8 years ago

I've been waiting for the work on #5658 to be finished before merging [11831:11832]. I should have the next round of changes posted for review later today.

I'm not sure what happened with [11853]. Maybe I had previously merged [11831:11832] in the working copy and reverted all of the changes except the prop change on the root of the working copy, and then inadvertently captured that change while merging [11852].

comment:8 by Ryan J Ollos, 8 years ago

… although there doesn't seem to be a good reason to not merge [11831] now. I'm not sure why I stopped at [11830] while doing the merges [11844:11849]. I should have stopped at [11831], only deferring the merge of [11832]. I'll merge [11831] to the trunk now and remove [11832] from the svn:mergeinfo property.

Last edited 8 years ago by Ryan J Ollos (previous) (diff)

comment:9 by Ryan J Ollos, 8 years ago

Resolution: fixed
Status: reopenedclosed

Should be fixed with [11902] and [11903]. Tests on the trunk are passing for me now.

I did yet another stupid thing, accidentally prefixing the log message in [11902] with 1.0.2dev. It seems that I can't fix this though,

$ svn propedit -r 11902 --revprop svn:log https://svn.edgewall.org/repos/trac

Revprop change blocked by pre-revprop-change hook (exit code 1) with output:
Changing *any* revision properties prohibited for now

Modify Ticket

Change Properties
Set your email in Preferences
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment

E-mail address and name can be saved in the Preferences .
Note: See TracTickets for help on using tickets.