Page MenuHomePhabricator

Should novices patches run full tests?
Closed, ResolvedPublic


(ping @Xqt, @Dalba, @zhuyifei1999)

I noticed novices patches are only running docker. Not nose-docker, nose34-docker or doc-docker test. Is there a reason for it or should we ask for full test suite also for them?

Event Timeline

Dvorapa created this task.Mar 5 2019, 9:43 AM
Restricted Application added subscribers: pywikibot-bugs-list, Aklapper. · View Herald TranscriptMar 5 2019, 9:43 AM
Xqt added a comment.Mar 5 2019, 4:23 PM

Don't know. Maybe this was introduced as we had 7 or more tests years ago. I think newbees need some help by experienced users at the beginning instead of failing tests.

Historically, CI was insecure. The ability to run full tests means that you can run arbitrary code on it, and historically CI test runners were not as 'isolated' as they currently are, so you had all sorts of opportunities to hijack the CI servers... a whitelist was needed. This was worked on in the CI isolation project, and now we do have nice isolation. I don't know why the whitelist was not removed, considering it was part of the long term plan. (perhaps to prevent, say DOS attacks?)

Dvorapa updated the task description. (Show Details)EditedMar 5 2019, 9:10 PM

I think newbees need some help by experienced users at the beginning instead of failing tests.

Makes sense

But perhaps they should run on every patchset and not only on the first one at least?

Xqt added a comment.EditedMar 23 2019, 12:27 AM

Since the last gerrit outage no test where done for CR+1 users. I guess this was introduced to prevent Jenkins running thousands of jobs after patches where vandalised. I think this would be ok for me. In a first step hints can be given by reviewer which is more readable than the test log. And CR+2 reviewers can “recheck” the code during the review process when appropriate.

I don’t think that we will go back to the previous state as patches where tested automatically since this vandalism impact and we can close this task or maybe have to close it due to this.

Xqt added a comment.Jun 23 2020, 9:13 AM

I think we can close this request and keep this status quo.

Xqt closed this task as Resolved.Jun 23 2020, 11:45 AM
Xqt claimed this task.