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

Trimming module name from classname #148

Open
daenney opened this issue Oct 26, 2022 · 4 comments
Open

Trimming module name from classname #148

daenney opened this issue Oct 26, 2022 · 4 comments
Labels

Comments

@daenney
Copy link

daenney commented Oct 26, 2022

Looking at the generated JUnit, the classname attribute ends up as <module>/<...>/<package name>. So for example it'll be github.com/jstemmer/go-junit-report/internal/junitreport. This makes the classname rather long and verbose and generally has the repeating domain.tld/something prefixed everywhere. In most cases this isn't very useful information and in some cases this breaks some JUnit reporting UI a bit (Gitlab renders this as a table and cells get super wide as a consequence).

Having searched around a bit, it seems that in general (at least for Java), the classname is just that, the class, not the full import path. It's supposed to be the function under test, but we don't know that in Go so I understand why the choice was made to encode the package in the attribute instead. But I'm wondering if we can provide an option to chop the module declaration off the classname, so you'd end up with classname="internal/junitreport" instead?

There doesn't appear to be an actual standard for this, but looking at the XSD for Ant a mandatory package key exists. JUnit 5 also has that field, but as optional instead. I'm wondering if that field could then be used to contain the module name instead?

@jstemmer
Copy link
Owner

jstemmer commented Nov 1, 2022

Thanks for the issue. I don't really know where people generally use go-junit-report, so it's great to get feedback like this.

I've had a look at how Gitlab shows JUnit report results and it indeed looks unnecessarily verbose, especially when module names are long. In v1 the classname used to be just the final component of the package path, but I intentionally changed it in v2 to improve how results are shown in Jenkins. From what I remember the classname used to be the full classname (including package), but I see you pointed out the package field so this may have changed. I'm sure there are plenty of other tools that each have their own way of rendering junit reports, so it probably make sense to make this configurable.

I don't think we know exactly which part of the package path is the module root. We could strip the common prefix among all packages as an approximation, but it might not always be correct. Another problem is that the last component in a package path is not always the same as the package name. Looking at the module root of go-junit-report for example (if it had any tests), we'd get v2 as the classname instead of the expected main.

I'll have to think a bit about what the best way would be address this and how it could be configured. Any thoughts?

@daenney
Copy link
Author

daenney commented Nov 2, 2022

If it's possible to get the module name somehow, then at least in theory it should be safe to trim that as a prefix. If you then end up with an empty string for classname, we would know we've hit main so you could substitute in the string main? This does wade a bit into hand-wavy heuristics so this behaviour should probably be behind some flag that's not active by default.

Since whoever invokes the report generator probably knows or could get to the module name, something simple like -trim-module-name="github.com.../v2" might do in practice. It's not the most elegant of things, but it would work reliably and would work in cases people don't want the whole prefix trimmed. If we can assume the reporter is called from a directory containing the source checkout then it could infer it itself but then you'd need a boolean flag to still indicate you want it trimmed.

Personally having to explicitly pass in this data is not an issue and it avoids having to pre-process the go test output by hand to remove the module name. That's not a terrible thing to have to do though so we might consider that the solution instead. I'd have to handle the empty string case in some shell script matching which will probably go wrong in interesting ways but not impossible. I can't quite figure out how I'd then use -p to add the package attribute on the testsuite though.

@jstemmer
Copy link
Owner

If you then end up with an empty string for classname, we would know we've hit main so you could substitute in the string main?

Unfortunately it's not this easy. I used main as an example, but the package name could really be anything. I guess we could fallback to for example using part of the path, but then how do we decide what to trim off and what not.

I was curious what other languages used as their classname (if they even decided to fill in this attribute), so had a quick look around. Java+junit uses the full package name + class name and eslint appears to use the full path but omit the file extension. It seems this is more common than I thought, I'm not yet entirely convinced this should be solved within go-junit-report at the moment. It would be easy to say that this should be solved in the code that displays the results from the report, but I'm also aware that this is probably not something that's likely to be fixed anytime soon.

An alternative you could consider is to post-process the generated XML. It's not the most elegant solution, but it might be easier than attempting to pre-process the go test output.

@daenney
Copy link
Author

daenney commented Nov 11, 2022

Fair point about the post-processing of the report, I hadn't actually thought of that. I'll give that a go. Should be fairly easy to define some transformations to apply to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants