Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
On Thu, 4 Jan 2024, 5:23 am Sandro Santilli, wrote: > On Mon, Nov 27, 2023 at 05:09:07PM +1000, Nyall Dawson via QGIS-PSC wrote: > > On Mon, 27 Nov 2023, 4:56 pm Alessandro Pasotti, > wrote: > > > > > https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 > > > comment the instructions say: > > > > > > "The full test report (included comparison of rendered vs expected > > > images) can be found under the 'Checks' tab - 'QGIS tests', > > > 'Artifacts' section as test-results-5." > > > > > > But I couldn't find any test-results-5 in the artifacts section > > > > That's caused by a limitation in GitHub API. > > Would it be possible for that comment to contain instruction about how > to run that test locally ? Helping developers running failing tests > locally would reduce load on shared infrastructure. > Good idea, but unfortunately non trivial to execute ☹️ Obstacles are: - We need the ctest name for the test so that associated ctest -R "^testname\$" command can be determined to run just one test file. - We don't have access to the ctest name for the current test being executed during the test. So we'd have to **hardcode** these into each test individually. (There's unfortunately no fixed pattern used to specify the ctest test name from the actual test source file name, so we can't use qtest methods or macros which give us the source file name here) - If we want to generate a command which will run just ONE function from the test, then we hit against https://gitlab.kitware.com/cmake/cmake/issues/20470 . (This could possibly be worked around using a variation on https://stackoverflow.com/a/77084304) So not impossible to do, but also a considerable amount of work. If someone else takes this on I'm happy to review. 👍 Nyall ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
On Mon, Nov 27, 2023 at 05:09:07PM +1000, Nyall Dawson via QGIS-PSC wrote: > On Mon, 27 Nov 2023, 4:56 pm Alessandro Pasotti, wrote: > > > https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 > > comment the instructions say: > > > > "The full test report (included comparison of rendered vs expected > > images) can be found under the 'Checks' tab - 'QGIS tests', > > 'Artifacts' section as test-results-5." > > > > But I couldn't find any test-results-5 in the artifacts section > > That's caused by a limitation in GitHub API. Would it be possible for that comment to contain instruction about how to run that test locally ? Helping developers running failing tests locally would reduce load on shared infrastructure. --strk; ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
On Mon, 27 Nov 2023 at 17:21, Laurențiu Nicola wrote: > On Mon, Nov 27, 2023, at 09:09, Nyall Dawson via QGIS-Developer wrote: > > That's caused by a limitation in GitHub API. We can't retrieve the > workflow run id in an action, so that link will always just point to the > most recent workflow run for the PR. > > > Hi Nyall, > > I'm not sure if that's the one you're missing, and you might be aware of > it, but there's a run_id field in the > https://docs.github.com/en/actions/learn-github-actions/contexts#github-context > . > Thanks! This got me thinking about another approach to take... and we've finally got there: https://github.com/qgis/QGIS/pull/55423#issuecomment-1829008969 Now there's a direct link to the test report zip file in the comment! Nyall > > Laurentiu > > ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
On Mon, Nov 27, 2023, at 09:09, Nyall Dawson via QGIS-Developer wrote: > That's caused by a limitation in GitHub API. We can't retrieve the workflow > run id in an action, so that link will always just point to the most recent > workflow run for the PR. Hi Nyall, I'm not sure if that's the one you're missing, and you might be aware of it, but there's a run_id field in the https://docs.github.com/en/actions/learn-github-actions/contexts#github-context. Laurentiu ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
On Mon, Nov 27, 2023 at 8:09 AM Nyall Dawson wrote: > > > > On Mon, 27 Nov 2023, 4:56 pm Alessandro Pasotti, wrote: >> >> Hi Nyall, >> >> good news, thank you for this improvement! >> >> Just a quick question, in the linked PR: >> https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 >> comment the instructions say: >> >> "The full test report (included comparison of rendered vs expected >> images) can be found under the 'Checks' tab - 'QGIS tests', >> 'Artifacts' section as test-results-5." >> >> But I couldn't find any test-results-5 in the artifacts section, there are >> only: >> >> Artifacts >> build-22.04-qt5.tgz >> build-38-qt6.tgz > > > That's caused by a limitation in GitHub API. We can't retrieve the workflow > run id in an action, so that link will always just point to the most recent > workflow run for the PR. > > And in this case I reverted the change causing a test failure, so the most > recent workflow doesn't have the failure report. > > Hope that makes sense! > Yes, thanks for clarifying. -- Alessandro Pasotti QCooperative: www.qcooperative.net ItOpen: www.itopen.it ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
On Mon, 27 Nov 2023, 4:56 pm Alessandro Pasotti, wrote: > Hi Nyall, > > good news, thank you for this improvement! > > Just a quick question, in the linked PR: > https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 > comment the instructions say: > > "The full test report (included comparison of rendered vs expected > images) can be found under the 'Checks' tab - 'QGIS tests', > 'Artifacts' section as test-results-5." > > But I couldn't find any test-results-5 in the artifacts section, there are > only: > > Artifacts > build-22.04-qt5.tgz > build-38-qt6.tgz > That's caused by a limitation in GitHub API. We can't retrieve the workflow run id in an action, so that link will always just point to the most recent workflow run for the PR. And in this case I reverted the change causing a test failure, so the most recent workflow doesn't have the failure report. Hope that makes sense! Nyall > > Cheers. > > > On Mon, Nov 27, 2023 at 5:29 AM Nyall Dawson via QGIS-PSC > wrote: > > > > Hi PSC, > > > > I'm happy to announce that this grant is now complete! > > > > While the original proposal was explicitly stated to be a research > project with no guarantees of success, the end result is predominantly a > success (with some limitations!) > > > > You can see the new failure handling in action in this PR: > https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 > > > > What we have now is that any tests which fail a rendering comparison > will write a descriptive comment to the PR, as shown in the above link. The > comment details which render tests failed, where they are in the code, and > includes some helpful pointers to downloading the full test report and the > QGIS developer documentation. > > > > Originally, I hoped to link directly to the full test report or include > it as an attachment to the comment. Unfortunately this is NOT possible > given the current Github API. There's a bunch of notes I've added to the > initial comment in https://github.com/qgis/QGIS/pull/54906 which link to > the limitations / feature requests on Github's side, so we can monitor the > situation and further improve the reports if/when Github add this > functionality. > > > > As well as the above described improvements on the CI side, I've also > implemented lots of improvements in running the tests locally and how the > render test reports are generated and presented to developers! > > > > Thanks for making this possible! > > > > Nyall > > > > > > > > > > > > ___ > > QGIS-PSC mailing list > > qgis-...@lists.osgeo.org > > https://lists.osgeo.org/mailman/listinfo/qgis-psc > > > > -- > Alessandro Pasotti > QCooperative: www.qcooperative.net > ItOpen: www.itopen.it > ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
Re: [QGIS-Developer] [Qgis-psc] QGIS grant report: Improve test result handling on QGIS CI
Hi Nyall, good news, thank you for this improvement! Just a quick question, in the linked PR: https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 comment the instructions say: "The full test report (included comparison of rendered vs expected images) can be found under the 'Checks' tab - 'QGIS tests', 'Artifacts' section as test-results-5." But I couldn't find any test-results-5 in the artifacts section, there are only: Artifacts build-22.04-qt5.tgz build-38-qt6.tgz Cheers. On Mon, Nov 27, 2023 at 5:29 AM Nyall Dawson via QGIS-PSC wrote: > > Hi PSC, > > I'm happy to announce that this grant is now complete! > > While the original proposal was explicitly stated to be a research project > with no guarantees of success, the end result is predominantly a success > (with some limitations!) > > You can see the new failure handling in action in this PR: > https://github.com/qgis/QGIS/pull/55417#issuecomment-1826995755 > > What we have now is that any tests which fail a rendering comparison will > write a descriptive comment to the PR, as shown in the above link. The > comment details which render tests failed, where they are in the code, and > includes some helpful pointers to downloading the full test report and the > QGIS developer documentation. > > Originally, I hoped to link directly to the full test report or include it as > an attachment to the comment. Unfortunately this is NOT possible given the > current Github API. There's a bunch of notes I've added to the initial > comment in https://github.com/qgis/QGIS/pull/54906 which link to the > limitations / feature requests on Github's side, so we can monitor the > situation and further improve the reports if/when Github add this > functionality. > > As well as the above described improvements on the CI side, I've also > implemented lots of improvements in running the tests locally and how the > render test reports are generated and presented to developers! > > Thanks for making this possible! > > Nyall > > > > > > ___ > QGIS-PSC mailing list > qgis-...@lists.osgeo.org > https://lists.osgeo.org/mailman/listinfo/qgis-psc -- Alessandro Pasotti QCooperative: www.qcooperative.net ItOpen: www.itopen.it ___ QGIS-Developer mailing list QGIS-Developer@lists.osgeo.org List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
