Skip to content

Filter testsuite#5865

Merged
lrineau merged 19 commits intoCGAL:masterfrom
maxGimeno:Testsuite-Filter_testsuite-maxGimeno
Jul 30, 2021
Merged

Filter testsuite#5865
lrineau merged 19 commits intoCGAL:masterfrom
maxGimeno:Testsuite-Filter_testsuite-maxGimeno

Conversation

@maxGimeno
Copy link
Contributor

Summary of Changes

Adds files and change scripts to run the Filter testsuite from github.

Release Management

  • Affected package(s):Scripts, Maintenance, Testsuite

# Ceci est le premier message de validation :

First step of action

# Ceci est le message de validation numéro 2 :

add ssh stuff

# Ceci est le message de validation numéro 3 :

Update github action

# Ceci est le message de validation numéro 4 :

Fix Logic

# Ceci est le message de validation numéro 5 :

Don't override the PATH !!!

# Ceci est le message de validation numéro 6 :

Download the known host too, so it is possible to add new IPs in it.

# Ceci est le message de validation numéro 7 :

Fix GIT path and better management of VERSION_NUMBER
add ssh stuff

Update github action

Fix Logic

Don't override the PATH !!!

Download the known host too, so it is possible to add new IPs in it.

Fix GIT path and better management of VERSION_NUMBER

fix autotest_cgal

Fixes for scripts
Add test collection script odifications for "skipped"

fixes on conditions
result-encoding: string
script: |
const userName = context.payload.comment.user.login
if(userName == 'maxGimeno') {
Copy link
Member

Choose a reason for hiding this comment

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

We'll have to adapt that logic! 😄

@lrineau
Copy link
Member

lrineau commented Jul 26, 2021

  • add the flags to have the "normal" warnings

@lrineau lrineau added the Not yet approved The feature or pull-request has not yet been approved. label Jul 27, 2021
@maxGimeno maxGimeno force-pushed the Testsuite-Filter_testsuite-maxGimeno branch from 0aaa2cc to 02152ab Compare July 28, 2021 09:37
Comment on lines +24 to +35
- uses: actions/github-script@v3
if: steps.check.result != 'stop'
id: get_label
with:
result-encoding: string
script: |
//get branch name and username
const pr_url = context.payload.issue.pull_request.url
const pr_content = await github.request(pr_url)
const label = pr_content.data.head.label
console.log(label)
return label
Copy link
Member

Choose a reason for hiding this comment

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

This step cannot be skipped, because a comment even does not have access to the full PR payload, and we need to request it using github.request.

runs-on: ubuntu-latest
steps:
- uses: actions/github-script@v3
if: github.event.comment.user.login == 'maxGimeno' && contains(github.event.comment.body, '/testme')
Copy link
Member

Choose a reason for hiding this comment

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

Here you use jobs.<job_id>.steps[*].if, and the conditional could be factorized to the job, using jobs.<job_id>.if, I think. I have not tested. I just read the doc.

@lrineau
Copy link
Member

lrineau commented Jul 30, 2021

I think we can merge this PR. The only files that could impact the CGAL testsuite are:

  • Testsuite/test/collect_cgal_testresults_from_cmake, and
  • Scripts/developer_scripts/autotest_cgal

and none of them have been modified since the last time this PR was tested (see #5865 (comment)).

@lrineau lrineau removed the Not yet approved The feature or pull-request has not yet been approved. label Jul 30, 2021
@lrineau
Copy link
Member

lrineau commented Jul 30, 2021

I still see possible improvement, like the factorization of copy-pasted files in Maintenance/test_handling/filter_testsuite/, but we can do that later.

Do you agree @maxGimeno?

@maxGimeno
Copy link
Contributor Author

Yes, let's merge it

@lrineau lrineau self-assigned this Jul 30, 2021
@lrineau lrineau merged commit eb30bb0 into CGAL:master Jul 30, 2021
@lrineau lrineau deleted the Testsuite-Filter_testsuite-maxGimeno branch July 30, 2021 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants