Commons Email
  1. Commons Email
  2. EMAIL-27

[email] further improvements to test code and removal of emails validation

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Nightly Builds
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: Other

      Description

      Changes list:

      Email
      ---------------------
      -Removed Email validation
      -changed (back) to getInstance due to security exceptions caused when testing
      using maven
      -In setFrom(String email, String name) changed the checking to
      StringUtils.isNotEmpty
      -Fixed some incorrect logic in testing for charset in:
      addTo(String email, String name)
      addCc(String email, String name)
      addBcc(String email, String name)
      addReplyTo(String email, String name)

      Mulitpart Email
      ---------------------
      -Corrected style errors involving imports
      -Added validation to disallow null message contents in setMsg(String msg)
      -Removed redundant if statment from send() to streamline the function
      -Added validation to disallow null attachements to attach(EmailAttachment
      attachment)
      -Added URL validation for attach(URL url, String name, String description,
      String disposition)
      -Added Datasource validation for attach( DataSource ds, String name, String
      description)
      -Removed redundant if statement from getPrimaryBodyPart() as the code was a
      duplicate of code inside init()

      EmailAttachmentTest
      ---------------------
      -Coding style changes only

      EmailTest
      ---------------------
      -Added test for the Get/Set session
      -Minor Coding style changes
      -Added exception test for the person param to all relevant address functions
      -Added the server port as a configurable var and added it as a static property
      -Added code to set the configured port during tests

      HtmlEmailTest
      ---------------------
      -Minor Coding style changes
      -Added the server port as a configurable var and added it as a static property
      -Added code to set the configured port during tests

      MulitpartEmailTest
      ---------------------
      -Minor Coding style changes
      -Added the server port as a configurable var and added it as a static property
      -Added code to set the configured port during tests
      -Removed the hard coded test url and added reference to the configuration
      class instead
      -Added test for init

      SimpleEmailTest
      ---------------------
      -added apache license
      -Minor Coding style changes
      -Added the server port as a configurable var and added it as a static property
      -Added code to set the configured port during tests

      EmailConfiguration
      ---------------------
      -added apache license
      -Minor Coding style changes
      -changed the test url to a active url

      MockEmailConcrete
      ---------------------
      -added apache license
      -Minor Coding style changes
      -added function to help test get/set session

      MockHtmlEmailConcrete
      ---------------------
      -added apache license

      MockMulitPartEmailConcrete
      ---------------------
      -added apache license
      -added test for init function

      MockHtmlEmailConcrete
      ---------------------
      -added apache license

        Activity

        Hide
        David Eric Pugh added a comment -

        Okay, I moved the code.. Unit tests still fail in the same places as before, so
        nothign broke worse! Please double check and reopen if I made a mess again.

        Show
        David Eric Pugh added a comment - Okay, I moved the code.. Unit tests still fail in the same places as before, so nothign broke worse! Please double check and reopen if I made a mess again.
        Hide
        Scott added a comment -

        Regarding the MultipartEmail patch:
        The duplicate code in init() and getPrimaryBodyPart() was a result of the patch
        in COM-1526 not being entirely applied. It should be removed from init() and
        included back in getPrimaryBodyPart(). I'd be happy to supply a patch if its
        helpful.

        Show
        Scott added a comment - Regarding the MultipartEmail patch: The duplicate code in init() and getPrimaryBodyPart() was a result of the patch in COM-1526 not being entirely applied. It should be removed from init() and included back in getPrimaryBodyPart(). I'd be happy to supply a patch if its helpful.
        Hide
        Corey Scott added a comment -

        please refer to my posting on the dev list as to why the tests where failing.

        Eric,
        Can you please tell me why my patches arent applying correctly for you, so
        that I can make your life easier please correcting whatever is wrong.

        Thanks,
        Corey

        Show
        Corey Scott added a comment - please refer to my posting on the dev list as to why the tests where failing. Eric, Can you please tell me why my patches arent applying correctly for you, so that I can make your life easier please correcting whatever is wrong. Thanks, Corey
        Hide
        Corey Scott added a comment -

        Created an attachment (id=13234)
        style changes and a few minor mods (should get style conflicts down to 4 and all tests passing again)

        Show
        Corey Scott added a comment - Created an attachment (id=13234) style changes and a few minor mods (should get style conflicts down to 4 and all tests passing again)
        Hide
        David Eric Pugh added a comment -

        We are getting there! I have gone ahead and integrated our own checkstyle config,
        based on the config used by commons configuration. Your fixes reduced the
        errors to 309, but the errors where bogus about varialbe names.. By putting our
        own in, we are down to 74. Can you run "maven site" and verify the same?

        Also, I am getting some unit test failures out of EmailTest.

        Lastly, almost everything applied except for a couple changes. Resync, and you
        should see where it didn't apply.

        Show
        David Eric Pugh added a comment - We are getting there! I have gone ahead and integrated our own checkstyle config, based on the config used by commons configuration. Your fixes reduced the errors to 309, but the errors where bogus about varialbe names.. By putting our own in, we are down to 74. Can you run "maven site" and verify the same? Also, I am getting some unit test failures out of EmailTest. Lastly, almost everything applied except for a couple changes. Resync, and you should see where it didn't apply.
        Hide
        Corey Scott added a comment -

        there was something wrong with the previously posted patches submitted for
        this and the style only bug.

        I have regenerated the patch and tested applying it, it all appears to work
        this time. I am not sure what was causing the problems with this before but I
        hope it is fixed now.

        Sorry for any inconvenence caused.

        Show
        Corey Scott added a comment - there was something wrong with the previously posted patches submitted for this and the style only bug. I have regenerated the patch and tested applying it, it all appears to work this time. I am not sure what was causing the problems with this before but I hope it is fixed now. Sorry for any inconvenence caused.
        Hide
        Corey Scott added a comment -

        Created an attachment (id=13231)
        this patch is cumlative of all other recently submitted patches

        Show
        Corey Scott added a comment - Created an attachment (id=13231) this patch is cumlative of all other recently submitted patches
        Hide
        Corey Scott added a comment -

        Created an attachment (id=13225)
        patch as described in bug

        Show
        Corey Scott added a comment - Created an attachment (id=13225) patch as described in bug

          People

          • Assignee:
            Unassigned
            Reporter:
            Corey Scott
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development