Specobject bugfixes#169
Conversation
kaklakariada
commented
Aug 4, 2018
kaklakariada
commented
- Fix location when importing specobjects
- Fix exported covers name
Codecov Report
@@ Coverage Diff @@
## develop #169 +/- ##
=============================================
+ Coverage 94.31% 94.31% +<.01%
Complexity 1004 1004
=============================================
Files 106 106
Lines 2743 2745 +2
Branches 231 231
=============================================
+ Hits 2587 2589 +2
Misses 98 98
Partials 58 58
Continue to review full report at Codecov.
|
| } | ||
| else | ||
| { | ||
| LOG.warning(() -> "Name '" + this.name + "' does not match legacy name pattern '" |
There was a problem hiding this comment.
This might potentially spam the log.
The warning is okay in cases where the error is rare though.
Longterm question: I thought about formalizing importer error collection. Instead of a regular text log a separate data structure that collects meta data about defect or spurious specification items? Opinions?
There was a problem hiding this comment.
Regarding error collection: 👍
| { | ||
| this.writer.writeStartElement("provcov"); | ||
| writeElement("linksto", coveredId.getName()); | ||
| writeElement("linksto", coveredId.getArtifactType() + ":" + coveredId.getName()); |
There was a problem hiding this comment.
I don't think this is correct since it will break in cases where people use existing ReqM documents without our proprietary <artifact-type>:<requirement name> convention.
There was a problem hiding this comment.
This only affects the export. The importer is unchanged. If we don't modify the exporter like this, the importer will use unknown as default which is also not good.
| public void setLocation(final String path, final int line) | ||
| { | ||
| this.location = Location.create(path, line); | ||
| this.setLocation(Location.create(path, line)); |
There was a problem hiding this comment.
👍
Ooops, that kind of design slip should not happen. whistles slightly embarrassed
| this.listener.setId(this.idBuilder.build()); | ||
| this.listener.endSpecificationItem(); | ||
| this.listener.setLocation(this.locationBuilder.build()); | ||
| this.listener.endSpecificationItem(); |
There was a problem hiding this comment.
Oh, that's a nasty one. Good job uncovering that!
| + " <providescoverage>\n" // | ||
| + " <provcov>\n" // | ||
| + " <linksto>covered</linksto>\n" // | ||
| + " <linksto>feat:covered</linksto>\n" // |
There was a problem hiding this comment.
That is what I meant earlier: existing documents will break. There must be a more robust solution that at least allows you to import a specobject and get the same back in export.
In absence of a better idea I suggest an exporter configuration switch. The current solution fixes one thing by breaking another.
| * the file content. | ||
| * @return an {@link InputFile}. | ||
| */ | ||
| public static InputFile forContent(final Path path, final String content) |
There was a problem hiding this comment.
Since it is under src/test: 👍
| @Test | ||
| public void testTraceContent() | ||
| { | ||
| final String content = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" // |
There was a problem hiding this comment.
Java Designers should finally make heredocs part of the language. :-)
|
Approved by @redcatbear, merging now. |
2 participants
