Fix coding standard issues

Please also use this issue to fix the last failing test, so it becomes green again in 1.x!

@Shreya_th any plan to entirely resolve this NOW? Otherwise please stop unassinging other users!

@Anybody I'm working on this .Just give me few minutes.

Thank you! You have all time you need ;) But unassigning someone shouldn't be done this way.

Hi @Anybody ,
I have fixed all the coding standard issues and failed test issues .Kindly review the patch file with attached screenshot as well.

Thank you.

@Shreya_th thanks, again as MR please.

Hi @Anybody ,
Created MR for coding standard issues and failing test .Kindly review the changes.

Thank you.

Thanks @Shreya_th, I left some comments. The two failing tests please still need to be solved.

Shreya_th → closed merge request !12

Shreya_th → reopened merge request !12

Tests are still failing sadly.

@Anybody will you please pointout in which file tests are failing or it will be easy for me to figure out failing tests issues if you will attach screenshot .

@Shreya_th see this link: https://www.drupal.org/pift-ci-job/2765598 → (red one here in the issue)

Hi @Anybody ,
There is 500 response error which means Internal server error. So, we can't fix those errors. Those errors can only be fixed by your side.

Another errors are happening because of that one error I think. Please check it.

Well, of course the reasons can be fixed here, but we surely need to find out, why that happens. Does the same happen when running the tests locally?

Hi, I have tested this at my local also but the errors didn't occur in my local. I think this is something that is at server side. Only a guy with the devops knowledge can help in this.

@Grevil: Could you take a short look, please to clarify the situation?

The last submitted patch, 8: Fixed-coding-standard-issues-3387512.patch, failed testing. View results →

Anybody → closed merge request !11

Merge blocked: fast-forward merge is not possible. To merge this request, first rebase locally.

Promoting this to major as a lot of issues simply have unrelated failing pipelines due to this, so I will try to get this one out of the way quickly.

As far as I can see, a lot of code has changed since we have opened this issue (nearly every file has a merge conflict). So I suppose it would be faster for us all if I would just open a second MR with the current 1.x branch and quickly resolve everything there.

lrwebks → closed merge request !12

Well, I first assumed that the test was failing because of load times (leading to inconsistent checks), but after letting the test sit for a whole 5 seconds after every search query, the error persists, and it is becoming incomprehensible to me. :P @grevil wanted to take over, so I will let him take a look at this.

@lrwebks does this also fail locally?

I don't think it's a task for @Grevil. Did you have a look into the test results locally and replayed the html files linked? That should show you, what goes wrong.

The text "CONTENT C" appears in the text of this page, but it should not.

Is a quite clear message, please first try to resolve it yourself.
Did you also replay the steps manually to ensure it's not simply expected behavior now?

Indeed, I have looked through the generated HTML files and tested it many times locally as well as manually (It definitely works with manual testing in the UI!). My impression is that the failure is related to the test environment itself and the fact that the results don't get loaded/unloaded immediately, but after a small delay due to JS (the test check might simply be too fast for listjs to load).
@Grevil asked me to let him take over to work on the failing test, so I'll assign this back to him.

anybody → changed the visibility of the branch 3387512-fixes-on-newer-branch to hidden.

anybody → changed the visibility of the branch 3387512-fixes-on-newer-branch to active.

All done, let's see! BTW from my perspective the required changes are obvious. Looking into the HTML test results shows that things already work perfectly, but the tests were wrong. listjs hides contents, so of course the're still in the DOM!

Automatically closed - issue fixed for 2 weeks with no activity.