-
Notifications
You must be signed in to change notification settings - Fork 579
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.
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
Fix change logic in get-changed-trilinos-packages.sh and PullRequestLinuxDriver.sh (#3133) #3258
Fix change logic in get-changed-trilinos-packages.sh and PullRequestLinuxDriver.sh (#3133) #3258
Conversation
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' At commit: commit a4f290fc95ee746bfee71f82a7bde5ad4c7db6b5 Author: Roscoe A. Bartlett <[email protected]> Date: Mon Jul 23 10:05:41 2018 -0600 Summary: Fix some doc typos (trilinos#3133)
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' At commit: commit 9b9822f5f2ab7183bdc47eec2193a5fbf6ab83d1 Author: Roscoe A. Bartlett <[email protected]> Date: Wed Aug 8 10:40:15 2018 -0600 Summary: Find ProjectCiFileChangeLogic.py for all projects (trilinos#3133)
…nos#3133) This addresses a defect in the usage of Trilinos/cmake/ProjectCiFileChangeLogic.py. Before, it was not getting found for Trilinos.
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
@william76, can you please review this updated PR? It fixes two sets of problems with the auto PR testing logic. |
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
So it looks like the Trilinos auto PR tester does not use the script
That is not the right set of enables for the changes in the PR. There are no changes to Belos or TpetraCore. |
@bartlettroscoe |
@william76, I just mean that |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
@trilinos/framework, The latest testing iteration for this PR shown here shows empty configure output and nothing else. Can someone look at the Jenkins logs to see what might have happened? Is there a way that non Trilinos framework people can directly view the Jenkins logs? |
@bartlettroscoe Ross, we are working on an improvement to the autotester to dump the last 100 lines of the log of each of the Jenkins runs into the Github Failed test report created by the AT. It's not the same as having actual Jenkins access, but it might help out. We need to do a bit more testing, but hopefully soon we can introduce it into the system. |
@allevin, could the AT just attach the entire Jenkins job output as a file attachment with a |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
Now the Jenkins job output should be able to show what packages will be enabled even before the ctest -S driver script is run.
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: 1 or more Jobs FAILED Note: Testing will normally be attempted again in approx. 2 Hrs 30 Mins. If a change to the PR source branch occurs, the testing will be attempted again on next available autotester run. Pull Request Auto Testing has FAILED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
@bartlettroscoe I looked into this a bit this morning... Basically it looks like the Github API does not allow uploading files as issue attachments (see this response from Github). I also don't see any way to do is via PyGithub. I will ping some of the developers at PyGithub and see if they are aware of anyway to do it. I suspect they don't want automated systems to dump huge files onto their servers. |
Status Flag 'Pull Request AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Using Repos:
Pull Request Author: bartlettroscoe |
@allevin, thanks for checking on this. That makes sense. Another option is to put the log file contents into a |
That's exactly what we are doing. |
Status Flag 'Pull Request AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - - This Pull Request Requires Inspection... The code must be inspected by a member of the Team before Testing/Merging |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
3 similar comments
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
All Jobs Finished; status = PASSED, However Inspection must be performed before merge can occur... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed these changes and they seem consistent with what was discussed at the TriBITS/Trilinos meeting this week. I approve.
Status Flag 'Pre-Merge Inspection' - SUCCESS: The last commit to this Pull Request has been INSPECTED AND APPROVED by [ jwillenbring ]! |
Status Flag 'Pull Request AutoTester' - Pull Request will be Automerged |
Merge on Pull Request# 3258: IS A SUCCESS - Pull Request successfully merged |
trilinos#3133, trilinos#3265) Now calling this script will select the correct set of file changes and correct set of enables as part the goals of trilinos#3133. I have tested this manally on several different PRs and verified that the correct set of enables are being selected. I did a few other things while editing the script: * The script can now be run on a different scratch Trilinos repo. This allows it to be manually tested without messing up the status of your main Trilinos git repo (see details in PR trilinos#3258) * The Trilinos git repo where the PR branch is merged is now automatically updated to the correct state, no matter what its prior state is before it is run. The only requirement is that the 'origin' remote is set correctly. None of the other local branches have to be exist or set correctly at all. See the new comments at the top. * Make all formating if statements the same with the same indentation. * Removed code that was not used or did nothing. Without this change, the auto PR tester will test more package that is it supposed to.
@trilinos/framework
Description
This PR contains two sets of commits.
The first set of commits fixes the behavior of the
get-changed-trilinos-packages.sh
script to apply the correct global change logic for Trilinos. This updates TriBITS snapshot and adds unit test to ensure that the logic in the fileTrilinos/cmake/ProjectCiFileChangeLogic.py
is getting correctly applied in the scriptget-changed-trilinos-packages.sh
used by the Trilinos PR tester. The unit testing ofget-changed-trilinos-packages.sh
was not strong enough.The second set of changes in the commit b2cccbf which passes in the correct SHA1 for selecting the set of changed files in the script
PullRequestLinuxDriver.sh
. See #3265.Motivation and Context
The TriBITS scripts called by
get-changed-trilinos-packages.sh
were not pickup up the logic inTrilinos/cmake/ProjectCiFileChangeLogic.py
and was instead likely using the logic inTribitsExampleProject/cmake/ProjectCiFileChangeLogic.py
(see TriBITSPub/TriBITS@9b9822f). As a result, PRs like #3251 were incorrectly triggering the enable of all Trilinos packages.Also the modification to
PullRequestLinuxDriver.sh
in PR #3218 was using the wrong base SHA1 and was resulting in testing more packages than needed for the PR in many cases (see #3265).The merge of this PR should complete #3133.
How Has This Been Tested?
I expanded the unit tests for
get-changed-trilinos-packages.sh
to ensure it ignores a change to a*.cmake
file under thecmake/std/adm/
directory. I also ran the script manually for a few PRs, including #3251, and saw that it is selecting the right packages to test (and no package in the case of PR #3251).To test this updated script
PullRequestLinuxDriver.sh
, I did the following.First, I created the separate dummy PR testing repo where the PR branch merges are done:
The I just ran the script
PullRequestLinuxDriver.sh
on this for a few real PRs.First, I tested this for the PR #3260:
That is the correct set of enables (i.e. no enables). That will save me huge amounts of time (and would have allowed PR #3260 to be merged immediately).
Then I tested this for a few other PRs ...
Testing for PR #3275:
That looks correct.
Testing for PR #2930:
That looks correct. (FYI @prwolfe)
Testing for PR #3258: (Prior to updating the branch and the PR)
That looks like the correct set of enables for the set of files that are being changed in that (this) PR.
Checklist