Kuali Rice Development
  1. Kuali Rice Development
  2. KULRICE-11491

Remove duplicated test resources from KRAD web framework

    Details

    • Type: Task Task
    • Status: Closed Closed
    • Priority: Critical Critical
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: 2.5
    • Component/s: Development
    • Security Level: Public (Public: Anyone can view)
    • Labels:
      None
    • Similar issues:
      KULRICE-9273Remove dash from Javadoc parameters in krad uif
      KULRICE-13522Create Web Tests for KRAD Maintenance Collection Features
      KULRICE-11395Correct failing unit tests in krad-web-framework
      KULRICE-9117Deprecate old formatters from core web, new formatter/property editor framework as part of krad data
      KULRICE-12274KEW allows duplicate searchable attributes to be added to a document
      KULRICE-10746Create Web Tests for KRAD Transactional Dialogs
      KULRICE-13402Create Web Tests for KRAD Maintenance Dialogs
      KULRICE-13446Remove Jenkins Job duplication using templates
      KULRICE-4889Move krad-web-framework classes into the krad-web-framework module
      KULRICE-12877ComponentFreemarkerTest makes references to krad web ftl
    • Rice Module:
      KRAD
    • KRAD Feature Area:
      UIF Component
    • KAI Review Status:
      Not Required
    • KTI Review Status:
      Not Required
    • Code Review Status:
      Not Required
    • Include in Release Notes?:
      Yes

      Description

      Based on the email thread below, work to reduce or eliminate duplication of downstream project resources for use as test resources by rice-krad-web-framework, without sacrificing test coverage.

      I resolved that problem with a maven reimport. I am still not grasping why we have to have a bunch of duplicate code and have to copy over the current stuff to the duplicate to make things work (effectively making the same changes on the same file twice). It is also really annoying during coding when pulling up resources by name and making sure you don't open the wrong one file since everything is copied there as well.

      Thanks,
      Brian Smith

      On 12/17/2013 2:27 PM, Mark Fyffe wrote:
      The FTL resources from krad-web need to be kept up to date in order for the tests to pass, so are a little different from the resources marked 2_4_M2. It should be pretty easy to use the maven dependency:unpack plugin or similar to share web resources from rice-krad-web if it could be made a dependency of rice-krad-web-framework. However, rice-krad-web currently has a dependency on rice-krad-web-framework. This is problematic since making adding rice-krad-web as a dependency of rice-krad-web-framework would introduce a circular chain.

      Since rice-krad-web provides only web resources only - no Java code or unit tests - would it be possible to eliminate the Java dependencies from this project? This would allow rice-krad-web to become a dependency of rice-krad-web-framework, and eliminate the need to synch up FTL in order to run rendering unit tests. I'll try this out locally and follow up.
      FWIW The "2_4_M2" resources and code are XML component definitions and supporting code. I suspect these will only need to be updated if/when impacting changes are made, any may be useful for detecting impacting changes while still doing focused work within the rice-krad-web-framework project. Are this resources a concern, or is it primarily the need to keep FTL in synch?

      The KRAD-UifDefaults.properties file provides Rice config parameters for building a mock environment for GlobalResources - see UifUnitTestUtils. Note that since JaxbConfigImpl (the normal config file format) is part of rice-core-impl, it cannot be included in rice-krad-web-framework, so simple properties files are used instead for the unit test config. Other than the different format, this is just a standard Rice config file. I'm not sure why this resource wouldn't be available for Brian - it is loaded using ClassLoader.getResourceAsStream(). Maven is able to execute the tests requiring this resource without trouble. Is this possibly an IntelliJ issue?
      Thanks,
      Mark

      On Tue, Dec 17, 2013 at 4:31 PM, Jerry Neal <jkneal@iu.edu> wrote:
      Mark,

      Couple more questions. Developers are finding they need to copy resources when making changes. For example, a change to an FTL. How could the tests continue to pass with new code but outdated resources?

      Second question, what is KRAD-UIDefaults.properties used for? Brian is getting an exception about not being able to find this class when running the unit tests.

      Thanks!
      Jerry

      From: Jerry Neal jkneal@iu.edu
      Sent: Tuesday, December 17, 2013 3:34 PM
      To: 'Mark Fyffe'
      Cc: 'Claus Niesen'; 'Peter Giles'; Brian Smith (bsmith83@ad3.ucdavis.edu); 'Erik Meade'
      Subject: RE: test milestone snapshot

      Copying Brian and Erik so they have this information.

      Jerry

      From: Mark Fyffe mark.w.fyffe@gmail.com
      Sent: Tuesday, December 17, 2013 2:55 PM
      To: Jerry Neal
      Cc: Claus Niesen; Peter Giles
      Subject: Re: test milestone snapshot

      The code and resources in the 2_4_M2 packages support ViewLifecycleTest, which is responsible for a significant portion of the UIF test coverage at this time. I copied existing resources in from krad-sampleapp, krad-web, and kns-impl in order to quickly generate a high volume of usable mock data for building full lifecycle unit tests based on real-world utilization patterns (KULRICE-10546). These tests include coverage of FreeMarker templates as well as Java code involved in the view/component lifecycles. The intent was not for the resources to be temporary, nor to keep in sync with the original source. I have not yet pruned out unused portions, and could still do so, but quite a bit is needed to support existing lifecycle unit tests, and more tests could still be added based on this data to improve coverage.

      Is there a way to reuse code and resources from krad-sampleapp, krad-web, and kns-impl for unit testing without a circular dependency, or a preferred approach for improving unit test (not IT) coverage without undue effort? In particular, it would be great if krad-web-framework could use krad-web directly for FreeMarker templates. Until there is an alternative, we will lose coverage by getting rid of these packages.
      Best,
      Mark

      On Tue, Dec 17, 2013 at 1:27 PM, Jerry Neal <jkneal@iu.edu> wrote:
      Hi Mark,

      We were wondering what the plan is for the milestone snapshot in the test package? Do you have a feel for when we might be able to get rid of that?

      Thanks,
      Jerry

        Issue Links

          Activity

          Hide
          Mark Fyffe (Inactive) added a comment -

          Added dependencies to rice-krad-sampleapp-web:

          • rice-krad-data test jar, to support running IT tests. Without this dependency a JPA error is seen on startup due to a missing class.
          • spring-test, to support ViewLifecycleTest

          Added ViewLifecycleTest under krad-sampleapp/web/src/it/java

          At present, the tests are failing due to a mismatch in config settings between the new and old locations. Once I have these passing in IT, I'll remove from rice-krad-web-framework

          Show
          Mark Fyffe (Inactive) added a comment - Added dependencies to rice-krad-sampleapp-web: rice-krad-data test jar, to support running IT tests. Without this dependency a JPA error is seen on startup due to a missing class. spring-test, to support ViewLifecycleTest Added ViewLifecycleTest under krad-sampleapp/web/src/it/java At present, the tests are failing due to a mismatch in config settings between the new and old locations. Once I have these passing in IT, I'll remove from rice-krad-web-framework
          Hide
          Kristina Taylor (Inactive) added a comment -

          Not sure why you're getting the errors, but those libraries and the test should NOT go in the rice-krad-sampleapp-web. They really need to go in krad-it. Is there a problem with putting it in there?

          Show
          Kristina Taylor (Inactive) added a comment - Not sure why you're getting the errors, but those libraries and the test should NOT go in the rice-krad-sampleapp-web. They really need to go in krad-it. Is there a problem with putting it in there?
          Hide
          Mark Fyffe (Inactive) added a comment -

          I've been looking into this issue again yesterday and this morning, and am still at a bit of a stand-still. I have moved the test to krad-sampleapp-web, and again to krad-it in my local 2.5 environment. From neither project am I finding the DD entries needed to build the views being tested, nor am I seeing a clear way to control configuration other than through an external config file in ~/kuali/test. I am reluctant to dig too deeply into the IT framework, making changes to accommodate ViewLifecycleTest that could have far-reaching impact on other IT tests. It seems we can work through the setup, but in revisiting this analysis again I have to question whether or not moving ViewLifecycleTest, or the other unit tests that cover DD components behavior, is the right approach.

          There are several facets of the tests developed in rice-krad-web-framework that I am not seeing an immediate possibility for using the IT framework:

          • Fast execution time in an isolated environment; in the unit test framework I put together for testing DD components and FTL templates, tests initialize in a few seconds and operate in an isolated environment using mock services. In the IT environment, fully configured services are in place and at best initialization time is close to 3 minutes. None of these tests use data services, nor do they make external calls to other impl modules; introducing these factors adds setup time as well as an environmental factor that could potentially complicate the runtime scenario the test is intended to cover.
          • Repeat runs with multiple configurations. ViewLifecycleTest runs each test twice: once in synchronous mode, then another time in asynchronous mode. When the test fails in async mode, but passes in synchronous, for example, that will indicate a thread-safety issue. In the IT environment, I am not seeing a clear way to modify configuration during automated runs without implementing changes deep within the IT framework that may or may not interfere with the intended purpose of IT. None of the configurations ViewLifecycleTest uses are default settings, so are not necessarily well suited for integration testing.
          • Unit test coverage is a key goal of ViewLifecycleTest - to ensure that the unit suite executes at least 25% of the common case scenarios for UIF components. For example one test, #testSanity, ensures that the dummy login form runs its lifecycle and renders without errors. Another facet to this setup is that it provides a means for writing unit tests that cover the Uif*.xml data dictionary definition files and KRAD FTL library. The purpose is not to test integration with other modules, or downstream projects, but to test the code directly in krad-web and krad-web-framework - this coverage is not picked up by Sonar since it is not Java code, but is a valuable check nonetheless. Whether or not the coverage is measured, IT tests do not contribute to unit coverage since they are not run by 'mvn clean install'.
          • Fast response and reaction time to test failures on the development side. Unless deliberately skipped, unit tests are run by developers during clean install, and are run within a few minutes of each commit by the CI unit test jobs. Due to the long execution time and large numbers of test cases, IT tests are left for automated runs and issues only addressed when QA identifies that a test has broken and opens an issue. Although the nature of the test failures is not always clear, and has been the source of confusion at times, as unit tests these have served on several occasions since being built to uncover issues impacting UIF components that could have gone undetected for much longer.

          The views imported from krad-sampleapp for use by ViewLifecycleTest are not intended as IT tests for the sample app; they are Labs views (not part of the main app), so are by nature test cases in and of themselves. The Labs views were built by developers for use by developers and are intended to exercise specific scenarios, for testing the behavior of UIF components. At this time, the views are not changing - the intent is for these views to participate in a wide variety of testing scenarios as control cases for monitoring the stability and performance of the KRAD UIF framework. For example, the same views tested by ViewLifecycleTest are also tested by nightly JMeter runs.

          I'm not sure of the right approach from here, but am inclined to suggest it may be more sensible to "promote" the labs that are kept stable for performance testing to resources directly within the rice-krad-web-framework project itself. They do not belong in the main resources tree, but there could be a benefit to creating another resource tree in krad-web-framework for housing test views. This tree would be available in rice-krad-web-framework, and available as well to krad-sampleapp-web as an upstream dependency, facilitating deployment for manual, functional, and performance testing. However, there are multiple challenges involved in eliminating "circular" dependencies for the benefit of unit testing data dictionary components where they are defined in rice-krad-web-framework, and this approach would only address one - for example, the DD components defined for UIF rely directly on KNS, which relies directly on KIM impl. The FTL library is defined in krad-web, which depends at compile time on krad-web-framework. There are several more examples. Unless these implicit downstream dependencies can be identified and reorganized, we will not be able to execute unit tests that use the data dictionary from krad-web-framework without a workaround; and unless unit tests that use DD can be written very little of the code in the web framework can be covered. I would suggest that the present workaround of pulling downstream dependencies as optional test resources (skipping the tests when resources are missing) is a good compromise for the near term.

          I hope this write-up helps clarify the purpose of ViewLifecycleTest, and the challenges involved in moving it to the IT suite. If we can reach consensus on short term steps, I am happy to help make those changes. For now, however, I think we are in better shape leaving ViewLifecycleTest in place as a unit test, and pulling the resources as available from KRAD sampleapp; skipping the tests in automated runs when sampleapp hasn't been built yet. This setup addresses the core issues reported on this JIRA: there are no duplicate source files or resources, and a clean build from a fresh source checkout is able to complete normally without errors.

          Show
          Mark Fyffe (Inactive) added a comment - I've been looking into this issue again yesterday and this morning, and am still at a bit of a stand-still. I have moved the test to krad-sampleapp-web, and again to krad-it in my local 2.5 environment. From neither project am I finding the DD entries needed to build the views being tested, nor am I seeing a clear way to control configuration other than through an external config file in ~/kuali/test. I am reluctant to dig too deeply into the IT framework, making changes to accommodate ViewLifecycleTest that could have far-reaching impact on other IT tests. It seems we can work through the setup, but in revisiting this analysis again I have to question whether or not moving ViewLifecycleTest, or the other unit tests that cover DD components behavior, is the right approach. There are several facets of the tests developed in rice-krad-web-framework that I am not seeing an immediate possibility for using the IT framework: Fast execution time in an isolated environment; in the unit test framework I put together for testing DD components and FTL templates, tests initialize in a few seconds and operate in an isolated environment using mock services. In the IT environment, fully configured services are in place and at best initialization time is close to 3 minutes. None of these tests use data services, nor do they make external calls to other impl modules; introducing these factors adds setup time as well as an environmental factor that could potentially complicate the runtime scenario the test is intended to cover. Repeat runs with multiple configurations. ViewLifecycleTest runs each test twice: once in synchronous mode, then another time in asynchronous mode. When the test fails in async mode, but passes in synchronous, for example, that will indicate a thread-safety issue. In the IT environment, I am not seeing a clear way to modify configuration during automated runs without implementing changes deep within the IT framework that may or may not interfere with the intended purpose of IT. None of the configurations ViewLifecycleTest uses are default settings, so are not necessarily well suited for integration testing. Unit test coverage is a key goal of ViewLifecycleTest - to ensure that the unit suite executes at least 25% of the common case scenarios for UIF components. For example one test, #testSanity, ensures that the dummy login form runs its lifecycle and renders without errors. Another facet to this setup is that it provides a means for writing unit tests that cover the Uif*.xml data dictionary definition files and KRAD FTL library. The purpose is not to test integration with other modules, or downstream projects, but to test the code directly in krad-web and krad-web-framework - this coverage is not picked up by Sonar since it is not Java code, but is a valuable check nonetheless. Whether or not the coverage is measured, IT tests do not contribute to unit coverage since they are not run by 'mvn clean install'. Fast response and reaction time to test failures on the development side. Unless deliberately skipped, unit tests are run by developers during clean install, and are run within a few minutes of each commit by the CI unit test jobs. Due to the long execution time and large numbers of test cases, IT tests are left for automated runs and issues only addressed when QA identifies that a test has broken and opens an issue. Although the nature of the test failures is not always clear, and has been the source of confusion at times, as unit tests these have served on several occasions since being built to uncover issues impacting UIF components that could have gone undetected for much longer. The views imported from krad-sampleapp for use by ViewLifecycleTest are not intended as IT tests for the sample app; they are Labs views (not part of the main app), so are by nature test cases in and of themselves. The Labs views were built by developers for use by developers and are intended to exercise specific scenarios, for testing the behavior of UIF components. At this time, the views are not changing - the intent is for these views to participate in a wide variety of testing scenarios as control cases for monitoring the stability and performance of the KRAD UIF framework. For example, the same views tested by ViewLifecycleTest are also tested by nightly JMeter runs. I'm not sure of the right approach from here, but am inclined to suggest it may be more sensible to "promote" the labs that are kept stable for performance testing to resources directly within the rice-krad-web-framework project itself. They do not belong in the main resources tree, but there could be a benefit to creating another resource tree in krad-web-framework for housing test views. This tree would be available in rice-krad-web-framework, and available as well to krad-sampleapp-web as an upstream dependency, facilitating deployment for manual, functional, and performance testing. However, there are multiple challenges involved in eliminating "circular" dependencies for the benefit of unit testing data dictionary components where they are defined in rice-krad-web-framework, and this approach would only address one - for example, the DD components defined for UIF rely directly on KNS, which relies directly on KIM impl. The FTL library is defined in krad-web, which depends at compile time on krad-web-framework. There are several more examples. Unless these implicit downstream dependencies can be identified and reorganized, we will not be able to execute unit tests that use the data dictionary from krad-web-framework without a workaround; and unless unit tests that use DD can be written very little of the code in the web framework can be covered. I would suggest that the present workaround of pulling downstream dependencies as optional test resources (skipping the tests when resources are missing) is a good compromise for the near term. I hope this write-up helps clarify the purpose of ViewLifecycleTest, and the challenges involved in moving it to the IT suite. If we can reach consensus on short term steps, I am happy to help make those changes. For now, however, I think we are in better shape leaving ViewLifecycleTest in place as a unit test, and pulling the resources as available from KRAD sampleapp; skipping the tests in automated runs when sampleapp hasn't been built yet. This setup addresses the core issues reported on this JIRA: there are no duplicate source files or resources, and a clean build from a fresh source checkout is able to complete normally without errors.
          Hide
          Kristina Taylor (Inactive) added a comment -

          Ok, sounds good, thanks for clarifying. Ideally this would be a unit test that would be separate from any published views and be in another module besides the sample app but it sounds like that is a lot of work.

          Show
          Kristina Taylor (Inactive) added a comment - Ok, sounds good, thanks for clarifying. Ideally this would be a unit test that would be separate from any published views and be in another module besides the sample app but it sounds like that is a lot of work.
          Hide
          Mark Fyffe (Inactive) added a comment -

          Marking as resolved for now.

          I've removed the partially implemented IT test from KRAD sampleapp.

          Show
          Mark Fyffe (Inactive) added a comment - Marking as resolved for now. I've removed the partially implemented IT test from KRAD sampleapp.

            People

            • Assignee:
              Mark Fyffe (Inactive)
              Reporter:
              Mark Fyffe (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 3 hours Original Estimate - 3 hours
                3h
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 1 day, 6 hours
                1d 6h

                  Structure Helper Panel