-
Notifications
You must be signed in to change notification settings - Fork 340
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
External storage - Part 1 #141
Conversation
…hading, please please do not pick a different groupId.
… step need not query the implementation just to determine UNSTABLE status.
…y removed the improper exception handling in JUnitResultsStepExecution.
* @param testName Test name. | ||
* @param errorStackTrace Error stack trace. | ||
*/ | ||
public CaseResult(SuiteResult parent, String testName, String errorStackTrace) { |
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.
these are just moved up so that all the constructors are together
src/test/java/hudson/tasks/junit/pipeline/JUnitResultsStepTest.java
Outdated
Show resolved
Hide resolved
src/test/java/hudson/tasks/junit/storage/TestResultStorageTest.java
Outdated
Show resolved
Hide resolved
Some tracing call ends up calling the toString and it breaks..
|
||
static boolean queriesPermitted; | ||
|
||
private final ConnectionSupplier connectionSupplier = new LocalConnectionSupplier(); |
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 see that this impl has been copied to https://github.com/timja/jenkins-junit-postgresql-plugin/blob/c83e4553ae625e5976f5addaf617402068601266/src/main/java/io/jenkins/plugins/junit/postgresql/PostgresTestResultStorage.java#L39; does it make sense to define a SQL-based implementation in junit-plugin/src/main/
? Or to define a junit-sql
plugin with that implementation and add to test
scope?
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.
Possibly tbh I’m not using any Postgres specific features so could just call it junit-sql plugin not Postgres.
It’s very convenient to be able to develop the pluggable storage from inside the junit plugin, possibly once it matures it could be removed from here and then the external reference implementation added as test scope
This is a critical piece, I think, as it is an expensive operation done by default on the job index page, and currently limited to (IIRC) 20 builds just to avoid trashing disk. I would consider defining an API sufficiently expressive for this sort of graph to be rendered efficiently to be part of any complete proof of concept. |
I'm going to proceed with merging to make iterating easier in smaller PRs, but feel free:
|
Subsumes #110
The trend graph and most of the test report work.
Although trend graph could probably be rewritten to not load runs and instead use a query
History graph needs rewriting but can be done standalone
Package results aren't being loaded correctly in the test report
Blue ocean test reporting needs checking at some point.
I plan to continue it in separate PRs