Skip to content
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

Add IssueTrackerTestsStatusReporter and related refactorings (trilinos./Trilinos#3887) #322

Conversation

bartlettroscoe
Copy link
Member

@bartlettroscoe bartlettroscoe commented Jun 6, 2020

Adds a new class IssueTrackerTestsStatusReporter and related refactorings that will be used by grover to report on the status of Issue Tracker's related tests.

I verified it works with 'grover' (after some workarounds in 'grover') .

Makes it more clear that testsetXX is a noun and not an action testSetXXX.
…eporter (trilinos/Trilinos#3887)

Now the refactored and renamed class TestsetReporter is ready to be moved to
CDashQueryAnalyzeReport.py to be reused.
The correct name of the function is assertExpectedNumColsFromCsvFile() not
"NumRows" since it is asserting the number of columns in the row, not the
number of rows.
@bartlettroscoe bartlettroscoe self-assigned this Jun 6, 2020
@bartlettroscoe bartlettroscoe force-pushed the tril-3887-test-summary-tables-refactor branch 2 times, most recently from f06c400 to 6699e4c Compare June 7, 2020 19:24
This is a working version of TestsetsReporter but it just needs some more
tests.

I factored out some unit test helper functions into the new file
CDashQueryAnalyzeReportUnitTestHelpers.py.
…#3887)

This now generates HTML that looks okay in GitHub comments.
)

* Now you can pass in your own custom title and you can decide what you want
  to say about it.

* Added semi-string test for TestsetsReporter.getTestsHtmlReportStr()

* Added a more complete test for the other testset cases so this is more
  comlete.
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.
…f_5'

I was confused about the purpose of that test.
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.
@bartlettroscoe bartlettroscoe force-pushed the tril-3887-test-summary-tables-refactor branch from 6699e4c to c4f121a Compare June 7, 2020 19:34
@bartlettroscoe bartlettroscoe changed the title WIP: Add IssueTrackerTestsStatusReporter and related refactorings (trilinos./Trilinos#3887) Add IssueTrackerTestsStatusReporter and related refactorings (trilinos./Trilinos#3887) Jun 7, 2020
@bartlettroscoe
Copy link
Member Author

There is not really any online testing right now since I have to disable Travis CI. So I will test locally and push with the checkin-test.py script.

Copy link
Member Author

@bartlettroscoe bartlettroscoe left a comment

Choose a reason for hiding this comment

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

Several things need to get changed here

tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Show resolved Hide resolved
tribits/ci_support/CDashQueryAnalyzeReport.py Outdated Show resolved Hide resolved
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

1 participant