-
Notifications
You must be signed in to change notification settings - Fork 572
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
Create automated tool to update ATDM Trilinos GitHub issues with current status of tests attached to those issues #3887
Comments
@fryeguy52, as we discussed yesterday, this would be a fun thing to start working and it would have a large impact in making sure that people follow up on ATDM Trilinos github issues and require less of our own personal time to do so. |
@fryeguy52, other just just PyGitHub, some other things to consider include: |
FYI: based on our conversation today, I just added the scope:
That would make it so that we would never need to manually re-open github issues that should not have been closed and it would make it so that we would not have to manually remove entires for the |
FYI: Joe Frye mentioned that Aaron L. has some code that pulls down GitHub Issue data and sends out summary emails. We need to ask Aaron to look at his scripts once we can get down to this. |
CC: @rmmilewi |
@rmmilewi, take a look at: to understand the entire ATDM Trilinos build process and where this fits in and especially: |
@rmmilewi, as discussed at our meeting just now, I will add the option |
Needed as part of trilinos/Trilinos#3887 Build/Test Cases Summary Enabled Packages: Enabled all Packages 0) MPI_DEBUG => passed: passed=361,notpassed=0 (1.40 min) 1) SERIAL_RELEASE => passed: passed=361,notpassed=0 (1.22 min) Other local commits for this build/test group: d9134b4
@rmmilewi, I added the you can get setup locally to run the driver script as:
That will produce the files:
You read these in Python as:
You want to group these by the 'issue_tracker_url' field the test dicts and then you want to create an HTML string table using the function or perhaps the function Let me know if you have questions. Otherwise, I can mock up how this might look. It is not much code to create the markdown and HTML that would go into the GitHub Issue comment. |
@rmmilewil, I think I want to change these file formats over to proper JSON. When I do that at some point, I will let you know. It should be pretty easy to adjust. |
@bartlettroscoe So, I'm sorting out the design and assessing the requirements based on the notes I took, the audio recording of our session, and what's described here on this issue. Could you provide me with a example copy of the data that you want the tool to read in? One that you generated, looked over, and confirmed was valid and as-expected? I know I can fetch this data myself, but I want another human in the loop on this. 😄 |
@rmmilewi, I will be in the loop as much as needed but it should be trivial to generate the data yourself as provided in the instructions at: However, here is the file promotedAtdmTrilinosTestData.py generated from the command:
That is only partial data from today but it gives tests in all of the different categories 'twiof', 'twip', 'twim' and 'twif' in the associated summary HTML email text Please make sure that you can run that driver on your local machine and generate this same data as well. Again, it should be trivial if you have Python 2.7+. Just download those files, remove the Have a look at that data and let me me know if you have any questions about it. See the basic instructions above. |
@bartlettroscoe It's trivial, I know, and I'll be generating plenty of data myself, but it's an important step in the process of stakeholder engagement for me that I have some agreed-upon, canonical data that captures the phenomena of interest. Same goes for any other data you think I may need (like that CSV file you showed me). In this case, either I can generate it myself and send it to you, or we can use the data you just generated, so long as I get a confirmation from you. Speaking of requirements, does |
@rmmilewi, with my current thinking the the tool and process being written in this issue should never need to look at those CVS files. All of the data needed should be present in the Python dicts for each of these tests. If there is missing data, then we need to add it.
The best way to do this is so "Hey Ross, what testing day looks to be a good day for running my tool?". That is is just a string YYYY-MM-DD passed in the
It is only currently being tested with Python 2. Making sure it works with Python 3 is not a lot of work. There is an overlap of Python 2 and 3 that allows a Python program to work with both. (I need to figure out how to add Python 3 testing to the Travis CI testing for TriBITS.) |
@bartlettroscoe Ah, gotcha! Good to know!
Not a bad idea. It might be worthwhile to keep me in the loop for any noteworthy test/bug-related events. It would be interesting to build up a dataset of that sort.
Cool! From the looks of it, I can interact with your tools through your scripts, so I'm not concerned about any compatibility issues between your code and mine. For my part in this I'm leaning towards making sure my code is Python-3-compatible. It's possible that elements of this codebase could be reused in the future, and seeing as Python 2 has reached end of life, I expect Python 3 to be the norm for future development. Miranda and I reached the same conclusion about the Traffic code we've been developing. Of course, I'm open to any ideas or preferences you might have. |
@rmmilewi, not entirely. The code the generates the HTML tables needs to be reused. Please don't write that stuff again. That will just make more code to maintain.
The reality is that as long as Python 2.7 is the default Python on SNL CEO RHEL7 systems, our Python tools will need to support Pythong 2.7 for many years to come. |
This class will get used by the 'grover' tool or any tool that will report on and/or take action about issue trackers based on the status of the tests assoicated with them. I did a few things to support this: * Allowed removal of <style> block not liked by GitHub Markdown (it is shown in raw form in a GitHUb Issue commnent). * Added support function getIssueTrackerAndAssertAllSame(). * Added 'cdash_testing_day' field to output testsLOD and useed in issue tracker report.
While working on trilinos/Trilinos#3887, I noticed faulty logic in determining global pass/fail related to 'twip' and 'twim' tests. The existance of these test categories should not result in a global failure. The assumption with 'twim' is that the tests were disabled (on purpose hopefully). To make this pass, I added the field TestsetInfo.existanceTriggersGlobalFail that defaults to 'True' but is set to 'False' for the 'twip' and 'twim' test-sets.
…3887) The name 'TestsetInfo' did not seem descriptive enough for me when reviewing this. This name makes me thing this type is describing an arbitary testset but it is really describing a particular category of tests. Therefore, I think the name 'TestsetTypeInfo' makes it more clear that this type is describing a particular type of test category.
This is tested indrectly through the tests for the clas IssueTrackerTestsStatusReporter. We don't want to maintain large complex redundant tests.
* Updated several comments. * Added unit tests for getStandardTestsetTypeInfo() including for error condition. * Updated tests for getTestsetAcroFromTestDict() and added check for error condition. * Added unit tests for getFullCDashHtmlReportPageStr() and updating the vertical whitespace so that it looks better. * Changed name from getIssueTrackerAndAssertAllSame() to getIssueTrackerFieldsAndAssertAllSame(). * Removed override of testsetAcroList used by class IssueTrackerTestsStatusReporter. Only issues with issue trackers should be passed into this class. * Changed name from getIssueTrackerAndAssertAllSame() to getIssueTrackerFieldsAndAssertAllSame(). * Changed name from createOverallCDashReportSummaryLine() to getOverallCDashReportSummaryLine().
…linos#3887) With the creation and usage of getFullCDashHtmlReportPageStr(), the HTML-specific vars in main() are no longer needed! I also added a newline at end of the last </p> in the top block of HTML text. This makes a separator to the bottom content.
CC: @rmmilewi I just merged the TriBITS PR TriBITSPub/TriBITS#322 to the TriBITS 'master' branch that created a new class IssueTrackerTestsStatusReporter. I plugged this into the 'grover' code in the MR: I manually posted what the content would look like as posted by 'grover' to a few Trilinos GitHub Issues:
There are a few issues that need to be fixed in 'grover' before we can deploy the first version. This is outlined in: The only one that needs to get fixed before we can deploy and initial version of this tool is to refactor the code to not copy the test dicts into a different data-structure and then copy them back. The data was being copied back incorrectly in some cases and it does not support the addition of new fields. Once that is fixed, I think we can deploy the first version that just updates the open issues once a week. |
CC: @rmmilewi A problem with this approach of just embedding the tables in the GitHub Issue comments as you can see in:
is that the test summary tables are hard to read. First, GitHub Issue comments are very narrow and you can't see all of the columns of the table and makes for very tall rows. Second, the colors are not shown which makes the tables harder to read. Therefore, it would be good to find an easy way for developers to view these tables in their full HTML spender. One approach might be to attach the HTML as a file to the GitHub Issue comment. I showed how this might look in: However, you can't directly attach and Another approach would be to post the HTML to website and then view it from there. We could do that with a GitHub pages site maintained with with a GitHub repo, for example. Or, we could just post it to the trilinos.org site. Just to show what that might look like, I manually posted: and put in a link to this from: That looks much better and is much more readable. I think we need to do something like this to make this information more readable and useful for developers. |
Below I log some details of how I tested this with 'grover' and the subtasks that I did in the last week. Details: (click to expand)(6/6/2020) I have this working with grover for the branch in MR: and the TriBITS PR: I tested this on 'crf450' with:
That provided the summary output file
I manually posted what the content would look like to a few Trilinos GitHub Issues:
Some stuff in grover needs to be fixed and described in: Completed tasks: (click to expand)
|
FYI: Meet with @rmmilewi yesterday to go over the current status of Grover towards the minimum viable product. There are the todos:
|
@bartlettroscoe @rmmilewi Thanks for setting this up -- it looks to be really useful. Just wanted to mention a small typo: the Grover message is missing the word "are" just after "there":
|
@jhux2, thanks for the catch! Should be fixed going forward. |
We now have finally deployed the first "Minimum Viable Product" that adds a single comment per open ATDM Trilinos GitHub issue as described in ATDV-365. The Jenkins project: is set up to run once a week at 7 AM on Mondays to post these comments. I sent out the following email to the Trilinos developers yesterday, and then I manually ran that above Jenkins project which resulting in 11 comments posted and the log output showed:
and posted the comments:
The next step is to update the comments so they give suggestions on if the issue can be closed since the issues are addressed. |
Just noticed these as I was reading the help message.
Just noticed these as I was reading the help message.
Related to:
|
This issue has had no activity for 365 days and is marked for closure. It will be closed after an additional 30 days of inactivity. |
CC: @fryeguy52, @trilinos/framework
Description
As part of the work to implement a Python tool to pull down, analyze and the summarize CDash build and test data in #2933, we realized that we had all of the information that would be needed to update Trilinos GitHub issues about the status of tests associated with Trilinos GitHub issues. The idea would be to add a GitHub issue comment containing tables showing the current status of the tests related to a GitHub Issue. An example of what this could look like is shown in #3579 (comment) and #3833 (comment) where I just manually copied and pasted the HTML-formatted tables and rows for those issues right into the the GitHub comments.
For #3579 on 11/14/2018, the results for 11/13/2018 might look like:
Test results for #3579 as of testing day 2018-11-13
Tests with issue trackers Passed: twip=2
Tests with issue trackers Missing: twim=3
NOTE: With all of the tests associated with this issue passing or missing (i.e. disabled), might this Issue be addressed and perhaps be closed?
Detailed test results: (click to expand)
Tests with issue trackers Passed: twip=2
Tests with issue trackers Missing: twim=3
So the idea is is that once that comment was added, the Trilinos developer responsible for the github issue could add a comment stating that this was not a randomly failing test so having this test pass or be disabled indicated that the issue could be resolved and then close the issue. No need to look at CDash directly, add in new CDash links, etc. Just comment and close. That would save a lot of human time.
So we would add these comments in the following cases:
Once a week just as a reminder of the current status of the tests related to this issue (so that Trilinos developers would not forget about the issue).
When one of the associated tests changed status (e..g went from passing to failing or failings to passing). (But not for frequent randomly failing tests or that would create a lot of spam updates).
When all of associated tests were were all passing or missing for X (e.g. 2) consecutive days, like shown above. (But not for any randomly failing tests or that would create a lot of spam.)
When all of the associated tests are all passing for the full acquired test history (e.g. 30 days) or are missing for frequent randomly failing tests. (But not for rare randomly failing tests.)
This should make it so that people don't need to manually check on the status of the associated tests for a GitHub issue. They could just let an automated system (that will be created in this Story) update the GitHub issue when something worth noting has occurred and when the issue might be closed.
Also, this tool could check that if all of the tests are passing and the GitHub issue was closed, then it could automatically remove the entries for that issue from a list of
*.csv
files. And, if it detects that tests are not all passing and the GitHub issue is closed, then it could automatically re-open the issue and provide a summary of the test results.Related
Tasks
cdash_analyze_and_report.py
tool to write detailed test information and history for all tests with issue trackers being monitored by the tool. [Done]update_github_issues_for_tests_status.py
that will update GitHub issues as described above given the output from thecdash_analyze_and_report.py
tool described above ... See grover_update_trilinos_github_issues_with_test_status.sh [Done]cdash_analyze_and_report.py
to provide thefail_frequency
field and put in output data-structure for each issue.CDashQueryAnalyzeReport.py
to provide pass/fail criteria for each issue and provide suggestions for when to close an issue based on if tests passing for X days matchingfail_frequency
criteria. (needs more analysis)*.csv
file for Issues that are closed and have met the passing criteria based onfail_frequency
logic (see above).The text was updated successfully, but these errors were encountered: