Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LPS-174358 FDS | Tab key navigation skips dropdown on filter label #3094

Closed
wants to merge 3 commits into from
Closed

LPS-174358 FDS | Tab key navigation skips dropdown on filter label #3094

wants to merge 3 commits into from

Conversation

kresimir-coko
Copy link
Collaborator

@kresimir-coko kresimir-coko commented Mar 7, 2023 •
edited
Loading

https://issues.liferay.com/browse/LPS-174358

In order to have the proper focus behaviour it was needed to pass a ClayButton component instead of the ClayLabel component as a trigger for the ClayDropDown.

I have also noticed a lot of SF inconsistencies in the frontend-data-set-web module, for example missing empty lines between sibling elements. Are we disabling our ESLint rules somehow in this module?

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf

@kresimir-coko kresimir-coko requested a review from markocikos March 7, 2023 09:55
@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:sf - 0 out of 1 jobs passed in 8 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 444e0ac8476dd04b9e685d83d08f391ec7b17b3f

Sender Branch:

Branch Name: LPS-174358
Branch GIT ID: da7ca552257408fb700e6201bacabd56aa7410d0

0 out of 1jobs PASSED
For more details click here.
     [exec] Generating types (23 of 31): item-selector-taglib
     [exec] Generating types (24 of 31): layout-admin-web
     [exec] Generating types (25 of 31): @liferay/object-js-components-web
     [exec] Generating types (26 of 31): @liferay/notification-web
     [exec] Generating types (27 of 31): @liferay/oauth2-provider-web
     [exec] Generating types (28 of 31): @liferay/object-dynamic-data-mapping-form-field-type
     [exec] Generating types (29 of 31): @liferay/object-web
     [exec] Generating types (30 of 31): product-navigation-simulation-device
     [exec] Generating types (31 of 31): segments-web
     [exec] Prettier checked 3 files
     [exec] 
     [exec] Done in 161.40s.
     [exec] 
     [exec] > Task :portalYarnCheckFormat
     [exec] Gradle build finished at 2023-03-07 10:02:18.078.
     [exec] 
     [exec] BUILD SUCCESSFUL in 2m 43s
     [exec] 3 actionable tasks: 2 executed, 1 up-to-date
     [exec] 
     [exec] See the profiling report at: file:///opt/dev/projects/github/liferay-portal/build/reports/profile/profile-2023-03-07-01-59-34.html
     [exec] A fine-grained performance profile is available: use the --scan option.
     [exec] 
[stopwatch] [run.batch.test.action: 3:37.851 sec]
     [echo] 
      [get] Getting: http://test-1-29/job/test-portal-source-format/4239//consoleText
      [get] To: /opt/dev/projects/github/liferay-portal/20230307020218148.txt
      [get] Error getting http://test-1-29/job/test-portal-source-format/4239//consoleText to /opt/dev/projects/github/liferay-portal/20230307020218148.txt
[beanshell] Created cache file in /tmp/jenkins-cached-files/1684514195.txt

BUILD FAILED

/opt/dev/projects/github/liferay-jenkins-ee/commands/build-common.xml:15081: The following error occurred while executing this line:

/opt/dev/projects/github/liferay-jenkins-ee/commands/build-test-portal-source-format.xml:336: The following error occurred while executing this line:

/opt/dev/projects/github/liferay-jenkins-ee/commands/build-test-portal-source-format.xml:338: The following error occurred while executing this line:

/opt/dev/projects/github/liferay-portal/build-test-batch.xml:7631: The following error occurred while executing this line:

/opt/dev/projects/github/liferay-portal/build-test-batch.xml:1428: The following error occurred while executing this line:

/opt/dev/projects/github/liferay-portal/build-test-batch.xml:47: /opt/dev/projects/github/liferay-portal/20230307020218148.txt doesn't exist

@liferay-continuous-integration
Copy link
Collaborator

markocikos
markocikos approved these changes Mar 7, 2023
View reviewed changes
Copy link
Collaborator

@markocikos markocikos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! ✨

SF error looks unrelated.

I typically make sure to have FDS in PR title, so I can easily search for relevant closed PRs on GitHub.

@kresimir-coko kresimir-coko changed the title LPS-174358 Frontend Data Set | Tab key navigation skips dropdown on filter label LPS-174358 FDS | Tab key navigation skips dropdown on filter label Mar 7, 2023
@kresimir-coko
Copy link
Collaborator Author

What about this I posted in the initial comment @markocikos ?

I have also noticed a lot of SF inconsistencies in the frontend-data-set-web module, for example missing empty lines between sibling elements. Are we disabling our ESLint rules somehow in this module?

@markocikos
Copy link
Collaborator

I have also noticed a lot of SF inconsistencies in the frontend-data-set-web module, for example missing empty lines between sibling elements. Are we disabling our ESLint rules somehow in this module?

AFAIK, we don't have a special ESLint rule for this. It is a rule for JSP tags, enforced by sourceFormat script. JSP tags have similar syntax to JSX, so I like to mirror the same rule. But no, we don't formally apply this to JS files. Both blank row and no rows are allowed between siblings.

@kresimir-coko
Copy link
Collaborator Author

I found out the exact behaviour and reported a bug to frontend-projects

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:stable - 28 out of 28 jobs passed

❌ ci:test:relevant - 74 out of 78 jobs passed in 2 hours 43 minutes

Click here for more details.

This pull is eligible for reevaluation. When this upstream build has completed, using the following CI command will compare this pull request result against a more recent upstream result:

ci:reevaluate:1397401_3430

Base Branch:

Branch Name: master
Branch GIT ID: 444e0ac8476dd04b9e685d83d08f391ec7b17b3f

Upstream Comparison:

Branch GIT ID: 1876e4b0e1e9b3f91be504092fcaec8e2393f6e1
Jenkins Build URL: EE Development Acceptance (master) - 4356 - 2023-03-06[12:35:24]

ci:test:stable - 28 out of 28 jobs PASSED
28 Successful Jobs:
    ci:test:relevant - 74 out of 78 jobs PASSED

    4 Failed Jobs:

    74 Successful Jobs:
      For more details click here.

      Failures unique to this pull:

      Failures in common with acceptance upstream results at 1876e4b:
      Test bundle downloads:

      @liferay-continuous-integration
      Copy link
      Collaborator

      @kresimir-coko
      Copy link
      Collaborator Author

      ci:reevaluate:1397401_3430

      @liferay-continuous-integration
      Copy link
      Collaborator

      CI is reevaluating the build with build ID: 1397401_3430 against the latest valid upstream results.

      @liferay-continuous-integration
      Copy link
      Collaborator

      Previous ci:test:relevantbuild was unsuccessfully reevaluated against more recent upstream results.

      ✔️ ci:test:stable - 26 out of 26 jobs passed

      ❌ ci:test:relevant - 74 out of 78 jobs passed in 2 hours 44 minutes

      Click here for more details.

      This pull is eligible for reevaluation. When this upstream build has completed, using the following CI command will compare this pull request result against a more recent upstream result:

      ci:reevaluate:1397401_3430

      Base Branch:

      Branch Name: master
      Branch GIT ID: 444e0ac8476dd04b9e685d83d08f391ec7b17b3f

      Upstream Comparison:

      Branch GIT ID: 1876e4b0e1e9b3f91be504092fcaec8e2393f6e1
      Jenkins Build URL: EE Development Acceptance (master) - 4356 - 2023-03-06[12:35:24]

      ci:test:stable - 26 out of 26 jobs PASSED
      26 Successful Jobs:
        ci:test:relevant - 74 out of 78 jobs PASSED

        4 Failed Jobs:

        74 Successful Jobs:
          For more details click here.

          Failures unique to this pull:

          Failures in common with acceptance upstream results at 1876e4b:

          @markocikos
          Copy link
          Collaborator

          Failures are repeating, may be related.

          @mateusralv
          Copy link

          ci:test:relevant

          @liferay-continuous-integration
          Copy link
          Collaborator

          ❌ ci:test:stable - 27 out of 28 jobs passed

          ❌ ci:test:relevant - 72 out of 77 jobs passed in 1 hour 43 minutes

          Click here for more details.

          Base Branch:

          Branch Name: master
          Branch GIT ID: 7167fb45ee3ac3dc7e131e36c7a8d06997a43cf3

          Upstream Comparison:

          Branch GIT ID: 7167fb45ee3ac3dc7e131e36c7a8d06997a43cf3
          Jenkins Build URL: EE Development Acceptance (master) - 4364 - 2023-03-08[05:07:06]

          ci:test:stable - 27 out of 28 jobs PASSED

          1 Failed Jobs:

          27 Successful Jobs:
            ci:test:relevant - 72 out of 77 jobs PASSED

            5 Failed Jobs:

            72 Successful Jobs:
              For more details click here.

              Failures unique to this pull:

              1. build-lib-versions-jdk8/0/0
                     [exec] * What went wrong:
     [exec] Execution failed for task ':apps:adaptive-media:adaptive-media-image-impl:generateLicenseReport'.
     [exec] > Unable to render com.github.jk1.license.ProjectData(project ':apps:adaptive-media:adaptive-media-image-impl', [com.github.jk1.license.ConfigurationData(compileInclude, [com.github.jk1.license.ModuleData(com.drewnoakes, metadata-extractor, 2.18.0, [com.github.jk1.license.ManifestData(metadata-extractor, 2.18.0, null, Drew Noakes, null, null, false)], [], [com.github.jk1.license.PomData(\$\{project.groupId\}:\$\{project.artifactId\}, Java library for extracting EXIF, IPTC, XMP, ICC and other metadata from image and video files., https://drewnoakes.com/code/exif/, , [com.github.jk1.license.License(The Apache Software License, Version 2.0, http://www.apache.org/licenses/LICENSE-2.0.txt)], null, [com.github.jk1.license.PomDeveloper(Drew Noakes, , https://drewnoakes.com)])], false)]), com.github.jk1.license.ConfigurationData(licenseReport, [])], [], [com.github.jk1.license.ModuleData(com.drewnoakes, metadata-extractor, 2.18.0, [com.github.jk1.license.ManifestData(metadata-extractor, 2.18.0, null, Drew Noakes, null, null, false)], [], [com.github.jk1.license.PomData(\$\{project.groupId\}:\$\{project.artifactId\}, Java library for extracting EXIF, IPTC, XMP, ICC and other metadata from image and video files., https://drewnoakes.com/code/exif/, , [com.github.jk1.license.License(The Apache Software License, Version 2.0, http://www.apache.org/licenses/LICENSE-2.0.txt)], null, [com.github.jk1.license.PomDeveloper(Drew Noakes, , https://drewnoakes.com)])], false)])
     [exec] 
     [exec] * Try:
     [exec] Run with --info or --debug option to get more log output. Run with --scan to get full insights.
     [exec] 
     [exec] * Exception is:
     [exec] org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':apps:adaptive-media:adaptive-media-image-impl:generateLicenseReport'.
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda\$executeIfValid\$3(ExecuteActionsTaskExecuter.java:186)
     [exec]   at org.gradle.internal.Try\$Failure.ifSuccessfulOrElse(Try.java:268)
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:184)
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:173)
              Failures in common with acceptance upstream results at 7167fb4:
              Test bundle downloads:

              @liferay-continuous-integration
              Copy link
              Collaborator

              @kresimir-coko
              Copy link
              Collaborator Author

              I think QA needs to update tests in order to get this green, as the tests are expecting a label class which is not a btn

              @mateusralv
              Copy link

              Hi @kresimir-coko I will create a ticket for the test fix to be done later so this PR will be forwarded faster. So the failures are not related, seems safe forward.

              Test that needs to be fixed: FDSFilters#OneFilterCanBeRemoved

              Testray comparison

              @kresimir-coko kresimir-coko mentioned this pull request Mar 10, 2023
              Closed
              @kresimir-coko
              Copy link
              Collaborator Author

              Manually forwarded to brianchandotcom#131463

              @markocikos
              Copy link
              Collaborator

              @kresimir-coko @mateusralv Sorry, but I closed the forwarded PR on Brian's fork. Is this really safe to manually forward? There is a legitimate failure in master. If the manually forwarded PR was merged, wouldn't it cause CI failures on all PRs related to FDS? Shouldn't we fix the test first, or at least quarantine it? /cc @manoelcyreno

              @markocikos markocikos reopened this Mar 10, 2023
              @kresimir-coko
              Copy link
              Collaborator Author

              Same concern that I had but Mateus' wording made it seem safe to forward 🤷

              @mateusralv
              Copy link

              Hi @markocikos yes most of the time we send the test fix along with the PR. In this case, I noticed that the failure occurs in only 1 FDS test, and we thought that this way would be faster and we could solve it after the PR is merged. But I can update this PR with the Fix if your prefer. What do you think?
              Cc: @manoelcyreno

              @manoelcyreno
              Copy link

              @mateusralv , can you update this PR, putting the test in "ignore" and adding a comment to fix this test in another ticket?
              https://issues.liferay.com/browse/LPS-178165 - Fix the poshi test impacted by the PR #3094

              @kresimir-coko
              Copy link
              Collaborator Author

              Closing in favor of #3115 which contains test updates, too

              @mateusralv mateusralv mentioned this pull request Mar 13, 2023
              Closed
              Closed
              @kresimir-coko kresimir-coko deleted the LPS-174358 branch April 19, 2024 05:57
              Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
              Reviewers

              @carloslancha carloslancha carloslancha left review comments

              @markocikos markocikos markocikos approved these changes

              Assignees
              No one assigned
              Labels
              ci:test:relevant - failure ci:test:sf - failure ci:test:stable - failure
              Projects
              None yet
              Milestone
              No milestone
              Development

              Successfully merging this pull request may close these issues.
              6 participants
              @kresimir-coko @liferay-continuous-integration @markocikos @mateusralv @manoelcyreno @carloslancha