Wicket
  1. Wicket
  2. WICKET-4112

WicketTester#startComponentInPage and WicketTester#assertModelValue have inconsistent behavior.

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.0, 1.5.1
    • Fix Version/s: 1.5.2
    • Component/s: wicket
    • Labels:
      None

      Description

      I try the following (in Pseudocode):

      tester = new WicketTester();
      tester.startComponentInPage(new Label("test","hello");
      tester.assertModelValue("test","hello"); --> throws

      java.lang.IllegalArgumentException: Component is not a container and so does not contain the path test.

      Funny as it is, to make this work, you have to do the following assertion:

      tester.assertModelValue("", "hello");

      The error lies in
      public Component getComponentFromLastRenderedPage(String path,
      final boolean wantVisibleInHierarchy)
      in Line: path = startComponent.getId() + ":" + path;

      I will provide a patch this evening.

      1. WICKET-4112.patch
        6 kB
        Andrea Del Bene
      2. 0001-WICKET-4122-WicketTester-uses-absolute-Path-to-refer.patch
        3 kB
        Martin Dilger
      3. wicket.zip
        31 kB
        Martin Dilger

        Activity

        Hide
        Martin Grigorov added a comment -

        Improved with r1180916.

        Now BaseWicketTester keeps meta data about the started component like a flag that indicates whether the component is instantiated by Wicket or not and uses it internally to decide how to construct the component path.
        It is transparent to the user - she always has to assert with: tester.assertXYZ("herComponentId", ...).

        @Andrea: thanks for your work on this!

        Show
        Martin Grigorov added a comment - Improved with r1180916. Now BaseWicketTester keeps meta data about the started component like a flag that indicates whether the component is instantiated by Wicket or not and uses it internally to decide how to construct the component path. It is transparent to the user - she always has to assert with: tester.assertXYZ("herComponentId", ...). @Andrea: thanks for your work on this!
        Hide
        Martin Grigorov added a comment -

        The problem is not completely solved.
        org.apache.wicket.util.tester.BaseWicketTester.startComponentInPage(Class<C>, IMarkupFragment) uses the artificial "testObject" as id for the Component that will be instantiated from the passed Class.

        I.e. if you do: startComponentInPage(MyPanel.class) then Wicket wont be aware how to find the components inside the Panel.

        Show
        Martin Grigorov added a comment - The problem is not completely solved. org.apache.wicket.util.tester.BaseWicketTester.startComponentInPage(Class<C>, IMarkupFragment) uses the artificial "testObject" as id for the Component that will be instantiated from the passed Class. I.e. if you do: startComponentInPage(MyPanel.class) then Wicket wont be aware how to find the components inside the Panel.
        Hide
        Andrea Del Bene added a comment -

        Improved patch. All tests went fine.

        Show
        Andrea Del Bene added a comment - Improved patch. All tests went fine.
        Hide
        Andrea Del Bene added a comment -

        I don't see it as a workaround, rather we are simply giving to method 'startComponentInPage' more informations. This kind of solution wouldn't break WICKET-1214 (no need to fix 'testLabel' assertions).
        Anyway there could be better solutions...

        Show
        Andrea Del Bene added a comment - I don't see it as a workaround, rather we are simply giving to method 'startComponentInPage' more informations. This kind of solution wouldn't break WICKET-1214 (no need to fix 'testLabel' assertions). Anyway there could be better solutions...
        Hide
        Martin Dilger added a comment -

        Well, could work (I did not investigate further), but it "feels" like a workaround. I´m not sure, whether this is a good idea. Other opinions?

        Show
        Martin Dilger added a comment - Well, could work (I did not investigate further), but it "feels" like a workaround. I´m not sure, whether this is a good idea. Other opinions?
        Hide
        Andrea Del Bene added a comment -

        Well, we could expand method 'startComponentInPage( component, pageMarkup)' adding a string parameter with the start component path. This parameter can be saved and used instead of 'startComponent'. Method 'startPanel' will pass panel id as start component path, while method 'startComponentInPage' will pass a null.
        See provided patch (I didn't tested yet!).

        Show
        Andrea Del Bene added a comment - Well, we could expand method 'startComponentInPage( component, pageMarkup)' adding a string parameter with the start component path. This parameter can be saved and used instead of 'startComponent'. Method 'startPanel' will pass panel id as start component path, while method 'startComponentInPage' will pass a null. See provided patch (I didn't tested yet!).
        Hide
        Martin Dilger added a comment -

        I spent some minutes to play around. I would be possible to reference tested components with their absolute paths (see the provided patch), but this would possibly break a whole lot of tests, that users just have fixed migrating to 1.5.

        I'm not sure, what would be a good / better way to fix that.

        Show
        Martin Dilger added a comment - I spent some minutes to play around. I would be possible to reference tested components with their absolute paths (see the provided patch), but this would possibly break a whole lot of tests, that users just have fixed migrating to 1.5. I'm not sure, what would be a good / better way to fix that.
        Hide
        Martin Dilger added a comment - - edited

        I agree, this conflicts and was introduced in WICKET-1214, but I think, it could be safe to remove since
        now we can use startComponentInPage which removes the indirection with ITestPanelSource, I will inspect on this.
        Any recommendations?

        Show
        Martin Dilger added a comment - - edited I agree, this conflicts and was introduced in WICKET-1214 , but I think, it could be safe to remove since now we can use startComponentInPage which removes the indirection with ITestPanelSource, I will inspect on this. Any recommendations?
        Hide
        Andrea Del Bene added a comment -

        IMHO we should consider to remove the following code

        if (startComponent != null)

        { path = startComponent.getId() + ":" + path; }

        inside BaseWicketTester.getComponentFromLastRenderedPage(String path,final boolean wantVisibleInHierarchy). This method shouòd use an absolute path to retrieve component from page.

        Show
        Andrea Del Bene added a comment - IMHO we should consider to remove the following code if (startComponent != null) { path = startComponent.getId() + ":" + path; } inside BaseWicketTester.getComponentFromLastRenderedPage(String path,final boolean wantVisibleInHierarchy). This method shouòd use an absolute path to retrieve component from page.
        Hide
        Martin Dilger added a comment -

        Quickstart with a failing test

        Show
        Martin Dilger added a comment - Quickstart with a failing test

          People

          • Assignee:
            Martin Grigorov
            Reporter:
            Martin Dilger
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development