Update insert optional data tests and update indices visitor. #1212

michaelcfanning · 2019-01-14T04:03:36Z

No description provided.

michaelcfanning · 2019-01-14T04:06:57Z

src/Sarif.Converters.UnitTests/FortifyFprConverterTests.cs

    {
+        public class FortifyFprConverterTestsFixture : DeletesOutputsDirectoryOnClassInitializationFixture { }


DeletesOutputsDirectoryOnClassInitializationFixture [](start = 55, length = 51)

This fixture deletes the outputs directory once per test session. Previously, we deleted this directory on executing every test case. Oops. This means that only the very last test failure was persisted to the directory. #Pending

Oh, nice, I will add that to SarifCurrentToVersionOneVisitorTests.

In reply to: 247381989 [](ancestors = 247381989)

Just want to be clear you understand this pattern. A fixture is called on a class with a public parameterless constructor. it is instantiated a single time on xunit identifying a class with one or more [Fact] or other attribute-derived tests. The test class itself cannot be the fixture class (as xUnit supports only one public ctor on a test class, due to the diversity of parameterizations of these things, I guess. Our fixture derives some computed data (i.e., the test output directory) from our test organization scheme. From the base class. I have recapped this minimal data computation and chosen to embed these fixtures as nested types with the actual test class (since they are not generally repurposable. The fixture must be named identically to the test class with the term 'Fixture' appended to it.

If you can think of anything cleaner/more elegant, let me know. The fixture itself can be provided, btw, to the test class ctor, with the result that it can be additionally parameterized when it is passed to each test case. This parameterization can be used most easily for test cleanup. You could imagine us simply placing a bool property on this thing, though ('HaveIDeletedTheOutputDirectoryYet' and having the first test case note that it's false, perform the delete itself, and set that bool to true. The test fixture is effectively just a bundle of static data with this approach. It would eliminate the requirement to define a nested type for each test (because we could put this standard operation into the file diffing base type). Thoughts?

In reply to: 247657934 [](ancestors = 247657934,247381989)

I'll read up on XUnit's "fixture" support and get back to you.

In reply to: 247661342 [](ancestors = 247661342,247657934,247381989)

I read the XUnit documentation of shared context. Now I know enough to ask:

Why do you derive a class from DeletesOutputsDirectoryOnClassInitializationFixture? I could understand it if the derived class added some functionality, but it doesn't. Why not just:

FortifyFprConverterTests : FileDiffingTests, IClassFixture<DeletesOutputsDirectoryOnClassInitializationFixture> { ... }

In reply to: 247662456 [](ancestors = 247662456,247661342,247657934,247381989)

Ok, I'm bouncing back and forth between commenting on the fixture and commenting on this class :-). I now see that the fixture has virtual properties that a FileDiffingTests-derived class could override in a bespoke derived fixture class. But in cases like this class, that do not override those properties or do anything else, I still think you should just mark them with IClassFixture<DeletesOutputsDirectoryOnClassInitializationFixture>.

In reply to: 247724801 [](ancestors = 247724801,247662456,247661342,247657934,247381989)

No, this won't work. These properties are initialized based on class naming convention. A single class can only represent the intended output location of a single suite. I'm just going to change the pattern entirely, stay tuned. Not this change.

In reply to: 247732570 [](ancestors = 247732570,247724801,247662456,247661342,247657934,247381989)

michaelcfanning · 2019-01-14T04:08:17Z

src/Sarif.TestUtilities/FileDiffingTests.cs


        protected virtual bool RebaselineExpectedResults => false;

-        protected virtual void RunTest(string resourceName)
+        protected virtual void RunTest(string inputResourceName, string expectedOutputResourceName = null)


RunTest [](start = 31, length = 7)

Here's how we handle one-to-many tests. Create an individual test referencing the common input, but generate an alternate output name. See the optional data visitor tests for a specific example. #Closed

So if an operation (such as a JSchema code gen from a schema) generates multiple outputs, we actually have to run the operation as many times as there are output files, even though all the outputs are generated each time? I'm not wild about that.

In reply to: 247382063 [](ancestors = 247382063)

glad to discuss further.

In reply to: 247659167 [](ancestors = 247659167,247382063)

michaelcfanning · 2019-01-14T04:09:16Z

        logs.All(

I changed some whitespace in this file, hence it is included. Oops. But observe this construct, which is currently failing. It is impossible to debug this test failure, need to improve this test for maintainability. #Pending

Refers to: src/Sarif.UnitTests/Processors/Log/SarifLogExtensionTests.cs:114 in e39fb52. [](commit_id = e39fb52, deletion_comment = False)

michaelcfanning · 2019-01-14T04:09:46Z

...arif.UnitTests/TestData/PrereleaseCompatibilityTransformer/ExpectedOutputs/NestedFiles.sarif

@@ -46,6 +46,22 @@
            "fileIndex": 3
          },
          "mimeType": "text/plain"


mimeType [](start = 11, length = 8)

This is the test update that somehow got clobbered recently. #ByDesign

michaelcfanning · 2019-01-14T04:10:30Z

src/Sarif.UnitTests/Visitors/InsertOptionalDataVisitorTests.cs

 using Microsoft.CodeAnalysis.Sarif.Writers;
 using Newtonsoft.Json;
 using Xunit;
 using Xunit.Abstractions;

 namespace Microsoft.CodeAnalysis.Sarif.Visitors
 {
-    public class InsertOptionalDataVisitorTests : FileDiffingTests
+
+    public class InsertOptionalDataVisitorTests : FileDiffingTests, IClassFixture<InsertOptionalDataVisitorTests.InsertOptionalDataVisitorTestsFixture>


InsertOptionalDataVisitorTests [](start = 17, length = 30)

converted this test suite to current pattern. suggest just looking at the new file rather than the diffs. will be most informative. #Closed

michaelcfanning · 2019-01-14T04:29:33Z

Larry, I'm mostly AWOL tomorrow. Can you massage this and merge according to your judgment? #Resolved

ghost · 2019-01-14T21:01:00Z

On it now.

In reply to: 453899975 [](ancestors = 453899975)

ghost · 2019-01-14T21:03:23Z

        logs.All(

Oh good heavens. It's elegant, I'll give it that ;-)

In reply to: 453898287 [](ancestors = 453898287)

Refers to: src/Sarif.UnitTests/Processors/Log/SarifLogExtensionTests.cs:114 in e39fb52. [](commit_id = e39fb52, deletion_comment = False)

michaelcfanning · 2019-01-14T21:16:41Z

        logs.All(

Indeed. Gratifyingly compact and the code is actually fairly readable. Unfortunately, it is impossible to debug on the failure case. :)

In reply to: 454160497 [](ancestors = 454160497,453898287)

Refers to: src/Sarif.UnitTests/Processors/Log/SarifLogExtensionTests.cs:114 in e39fb52. [](commit_id = e39fb52, deletion_comment = False)

ghost · 2019-01-15T00:12:56Z

src/Sarif/Visitors/UpdateIndicesVisitor.cs


-        public UpdateIndicesVisitor(IDictionary<string, int> fullyQualifiedLogicalNameToIndexMap, IDictionary<string, int> fileLocationKeyToIndexMap)
+        public UpdateIndicesVisitor(IDictionary<string, int> fullyQualifiedLogicalNameToIndexMap, IDictionary<FileLocation, int> fileLocationToIndexMap)


fileLocationToIndexMap [](start = 129, length = 22)

The one place this class is instantiated in product code, you now set this parameter to null. Is this a work in progress? Because as the product code stands in this branch, VisitFileLocation never does anything. #Pending

Yes. The VisitFileLocation in this visitor only applies to result matching. So this functionality is still pending. The prerelease transformer doesn't need this update to function.

In reply to: 247716347 [](ancestors = 247716347)

ghost · 2019-01-15T00:13:21Z

src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs

@@ -80,7 +80,7 @@ public static class PrereleaseCompatibilityTransformer
            {
                Debug.Assert(modifiedLog == true);
                transformedSarifLog = JsonConvert.DeserializeObject<SarifLog>(sarifLog.ToString());
-                var indexUpdatingVisitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, fileLocationKeyToIndexMap);
+                var indexUpdatingVisitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, null);


null [](start = 105, length = 4)

See my comment in UpdateIndicesVisitor.cs. #Closed

ghost · 2019-01-15T00:48:09Z

src/Sarif.TestUtilities/DeletesOutputsDirectoryOnClassInitializationFixture.cs

+    /// This class is invoked a single time on initializing an xunit class for testing. It deletes
+    /// any existing test output files that may have been produced by the previous run. This fixture
+    /// is required because individual tests in test classes result in an object instantiation for
+    /// each test case. The FileDiffingTests pattern, by design outputs a bundle of outputs to 


FileDiffingTests [](start = 28, length = 16)

Since this fixture is intended to support classes that derive from FileDiffingTests, why not do this:

class FileDiffingTests: IClassFixture<DeletesOutputsDirectoryOnClassInitializationFixture> { ... } class FortifyFprConverterTests: FileDiffingTests { ... }

... rather than marking each derived class as implementing IClassFixture. Does XUnit's reflection not find the interface if it's on a base class? #Resolved

This might relate to the comment I made on FortifyFprConverterTests, where I asked why you bothered to derive a class from DeletesOutputsDirectoryOnClassInitializationFixture. It's true that if you want to leave the flexibility for each derived test class to have its own fixture, then you can't put the marker interface on the base class.

If that was your reasoning, it's worth a comment here.

In reply to: 247725332 [](ancestors = 247725332)

Ok, I see now that DeletesOutputsDirectoryOnClassInitializationFixture has some virtual properties that derived classes can override, so you really do intend each derived class to have a shot at it. I've been reworking this comment, so I'll mention that.

In reply to: 247725737 [](ancestors = 247725737,247725332)

Ok, I updated this comment with my accustomed crystalline authorial voice :-)

In reply to: 247729832 [](ancestors = 247729832,247725737,247725332)

ghost · 2019-01-15T01:19:33Z

src/Sarif.UnitTests/Visitors/RebaseUriVisitorTests.cs

-
-            visitor.FileDataKeys.Count.Should().Be(4);
-            visitor.FileDataKeys.Where(k => k != null && k.StartsWith(uriRootText)).Count().Should().Be(3);
-            visitor.FileDataKeys.Where(k => k != null && k.StartsWith("#" + toolsRootBaseId + "#")).Count().Should().Be(1);


visitor [](start = 12, length = 7)

This test still fails. Was this change intended to fix it? #Closed

No, just an incidental change to allow for compilation: there are no more file data keys persisted by this visitor

In reply to: 247734541 [](ancestors = 247734541)

ghost · 2019-01-15T01:21:19Z

src/Sarif.UnitTests/Writers/SarifLoggerTests.cs

@@ -463,7 +463,7 @@ public void SarifLogger_ScrapesFilesFromResult()
            {
                string fileName = @"file" + i + ".cpp";
                string fileDataKey = "file:///" + fileName;
-                sarifLog.Runs[0].Files.Where(f => f.FileLocation.Uri.AbsolutePath.Contains(fileDataKey)).Any().Should().BeTrue();
+                sarifLog.Runs[0].Files.Where(f => f.FileLocation.Uri.AbsoluteUri.ToString().Contains(fileDataKey)).Any().Should().BeTrue();


AbsoluteUri [](start = 69, length = 11)

This change apparently fixes this test. But in your branch, three other SarifLoggerTests went from failing to passing (see the table I sent you), but I don't see any changes that would account for that. #ByDesign

I believe the null check on line 80 of run.cs resolved those.

In reply to: 247735706 [](ancestors = 247735706)

This picks up lgolding's fixes to the SarifCurrentToVersionOneVisitor and its tests.

…nFixture.

ghost

🕐

ghost · 2019-01-15T01:33:02Z

I have a couple of substantive comments and questions, about why some tests are now passing, and why one test is still failing, and why one product code class apparently isn't doing anything. :-) I also have a suggestion about the use of DeletesOutputDirectoryOnClassInitializationFixture (and another suggestion about its name).

I've merged files-array into your branch and pushed back to GitHub, so you should be able to pull and keep on trucking. #Pending

ghost · 2019-01-15T01:34:12Z

src/Sarif.TestUtilities/DeletesOutputsDirectoryOnClassInitializationFixture.cs

+    /// it wants to override the virtual TypeUnderTest or OutputFolderPath properties, but there seems
+    /// no good reason to do this.
+    /// </summary>
+    public abstract class DeletesOutputsDirectoryOnClassInitializationFixture


DeletesOutputsDirectoryOnClassInitializationFixture [](start = 26, length = 51)

This is an xUnit class fixture (which, as we now both know, does work on class initialization (and sometimes on class disposal). I suggest the name DeletesOutputDirectoryClassFixture (or even OutputDirectoryDeletingClassFixture). #WontFix

Our fixture only deletes on class initialization, so would prefer to retain the name. In any case, this fixture is going to be changed, I'm switching to a different pattern as this one is non-obvious and cumbersome to implement.

In reply to: 247737869 [](ancestors = 247737869)

michaelcfanning · 2019-01-15T21:12:57Z

On it, thanks for the merge.

In reply to: 454234368 [](ancestors = 454234368)

michaelcfanning · 2019-01-15T22:08:10Z

@lgolding, in fact, you were right to worry about the update indices visitor. I broke that test. Fixing separately, thanks for catching this.

* Fix up tests * Conversion to files array. WIP. Core SARIF component build complete except for SarifLogger tail. * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * Reduced files array build (#1191) * Sarif and Sarif.Converters now building * Files array (#1188) * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * DRY out converters to isolate shared code. * Restore essential change in schema that converts files dictionary to an array. * Simplify ShouldSerialize logic * Remove unused namespaces * Respond to PR feedback. * Respond to PR feedback * End-to-end build works. Now we can author core transformation and fix tests. (#1192) * Fix up merge from 'develop' branch. * Update supporting test code for processing checked in files. (#1202) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * Files array basic transform (#1205) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * WIP. Furhter progress * Fix up samples build * Fix up merge from basic transform branch * Mime type validation (#1206) * Fix up merge from basic transform branch * Fix up mime test * Start work on v1 <-> v2 transformation (#1208) * Restore TransformCommand and first (unaffected) unit test * Restore "minimal prerelease v2 to current v2" test. * estore "minimal v1 to current v2" test. * Restore remaining TransformCommand unit tests. * Uncomment v2 => v1 tests to observe failures. * Uncomment 'transform' command. * Restore MakeUrisAbsoluteVisitor tests (#1210) This change updates the visitor that expands URIs in the presence of `originalUriBaseIds`. Turns out there was technical debt here, because our tests provided `originalUriBaseIds` equivalents in the property bag (because we had no official SARIF slot for them). I did not notice this gap when we made the schema change to add `originalUriBaseIds`. * Get v2 -> v1 transform working with files array (#1211) Test failure count is down to 32; will be 28 when you merge your fix. There is not -- and never was -- a test case for fileLocations that use uriBaseId (never was one). I know for a fact that there is no code to support that case. You’ll see a comment to that effect in the code. I will take care of that next. Then I will move on to v1 -> v2 transform. As part of this change, the `SarifCurrentToVersionOneVisitorTests` are now based on the `RunTest` helper method from the base class `FileDiffingTests`. * Convert debug assert to exception to make test execution more deterministic (#1214) * Update insert optional data tests and update indices visitor. (#1212) * Update insert optional data tests and update indices visitor. * Delete speculatively needed file * Remove erroneous override of base visit method. * Rework summary comment on DeletesOutputsDirectoryOnClassInitializationFixture. * Update clang analyzer name. Flow converter log verification through JToken comparison. (#1213) * The multiool, core sarif, and validation test binaries now all pass (#1215) * The multiool, core sarif, and validation test binaries now all pass completely. * Remove unwanted assert that may fire during unit testing. * Merge from files-array * PR feedback. * PR feedback tweaks * Accept PR feedback from previous change. (#1216) Use LINQ IEnuemrable.Except in the unit test, which improves readability without compromising efficiency (because Except uses a Set to do its work in O(N) time). * Fix Sarif.Driver and Sarif.Functional tests. (#1217) * Fix Sarif.Driver and Sarif.Functional tests. * Restore test file * Fix Semmle tests and Fortify converter: all tests now pass. (#1218) * Sarif converters fixups (#1219) * Fix semmle tests and fority. * Final test fixups * Invoke appveyor for files-array branch.: (#1220) * Update SarifVersionOneToCurrentVisitor for run.files array (#1221) * Uncomment v1 -> v2 tests; 3/14 fail. * Move test data to locations expected by FileDiffingTests. * Fix up some IDE C#7 code cleanups. * Use FileDiffingTests helper. * Fix bug in FileDiffingTests that caused a test failure. * Remove default-valued argument from a call to RunTest. * Create basic files array Does not yet have fileIndex, parentIndex, or response file handling. * Revert incorrect change in FileDiffingTests. * Fix one unit test with spot fix to "expected" file. * Fix up some C#7 IDE warnings * Force update in FileDiffing tests to avoid deserialization errors from out of date "expected" files. * Fix missing "modified" flag sets in PreRelCompatTransformer. * Populate fileIndex in run.files array. * Fix unit test by fixing fileLocation creation. * Restore response file handling. * Populate fileIndex on fileLocations as appropriate. * Fix last test failure by reworking response file handling. * Feedback: Introduce transformer helper PopulatePropertyIfAbsent. * Feedback: Tighten platform-independent string compare. Also: - Reformat unit test lines. * Feedbakc: Revert FileDiffingTest change; downgrade affected test files to provoke transform * Basic rules transformation (except for v1 <-> v2 conversion) (#1223) * Basic rules transformation (except for v1 <-> v2 conversion) * Respond to very excellent PR feedback. * PR feedback * Add files array tests for nested files and uriBaseIds (#1226) * Add failing v1 -> v2 nested files test * Fix existing mis-handling of analysisTarget and resultFile. * Get nested files test case working. * Add failing v1 => v2 uriBaseId test. * Populate v2 uriBaseId. * Fix up expected v2 fileLocation.properties: test passes. * Enhance uriBaseIds test case. * Implement v2->v1 conversion for rules dictionary (#1228) * Notification rule index (#1229) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Notification rule index (#1230) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Missed feedback from previous PR (#1231) * Implement v1->v2 conversion for rules dictionary (#1232) * Partial implementation * Get last test working. * Somebody missed checking in a generated file. * Schema changes from TC #30 (#1233) * Add source language, fix rank default. * Adjust rank minimum to accommoodate default. * Fix broken test. * Remove unnecessary None items from project file. * PR feedback * Files array results matching (#1234) * WIP preliminary results matching * Restore results matching for files array * Add back autogenerated (unused) namespace directive

@fishoak

…#1264) * Fix tests that are broken in appveyor (#1134) * Properly persist run level property bags (#1136) * Fix #1138: Add validation rule: contextRegion requires region (#1142) Also: - Enhance the "new-style" verification so that we no longer require the file "Invalid_Expected.sarif". Each file can now contain a property that specifies the expected locations of all the validation results. * Prep for 2018-11-28 schema update. Remove run.architecture. (#1145) * Add run.newlineSequences to schema (#1146) * Mark result.message as required in the schema (#1147) * Mark result.message as required in the schema * Update release history with result.message breaking change. * Fix typo in testoutput. * Rename tool.fileVersion to tool.dottedQuadFileVersion (#1148) * Upgrade more validator functional tests (#1149) We apply the new functional test pattern to four more rules: - `EndColumnMustNotBeLessThanStartColumn` - `EndLineMustNotBeLessThanStartLine` - `EndTimeMustBeAfterStartTime` (which is misnamed, and in a future PR we will rename it to `EndTimeMustNotBeBeforeStartTime`) - `MessageShouldEndWithPeriod` In addition, we remove the test data for a rule that no longer exists, `HashAlgorithmsMustBeUnique` (which no longer applies because `file.hashes` is now an object keyed by the hash algorithm). Because there are so many properties of type `message`, exhaustively testing the rule `MessageShouldEndWithPeriod` revealed many holes in the visitor class `SarifValidationSkimmerBase`, which I plugged. As we have discussed, we should generate this class from the schema. After this, there are only two more rules to convert: - `UriBaseIdRequiresRelativeUri` - `UseAbsolutePathsForNestedFileUriFragments` ... but this PR is already large enough. * Remove 'open' from list of valid rule configuration default values. (#1158) * Emit column kind default explicitly for Windows SDK SARIF emit. (#1160) * Emit column kind default explicitly for Windows SDK SARIF emit. * Update release notes * More column kind test fixes * Change behavior to always serialize column kind. * Always serialize column kind * Finish validator functional test upgrade (#1159) * Rename rule EndTimeMustBeAfterStartTime => ...MustNotBeBefore... * Upgrade UriBaseIdRequiresRelativeUri tests. * Remove obsolete rule UseAbsolutePathsForNestedFileUriFragments. * Remove support for old test design. * Remove 'package' as a documented logical location kind in the schema. Documentation only change. (#1162) * Fortify FPR converter improvements + unit tests (#1161) * Improvements and corrections Populate originalUriBaseIds from <SourceBasePath> Populate tFL.kind from <Action type="..."> Add default node to result.locations * Add location annotation for Action elements with no type attribute * Support node annotations + uribasepath + test updates * Update FortifyTest.fpr.sarif * Add converter tests & assets + opportunistic code cleanup * PR feedback * Logical locations dictionaries to arrays (#1170) The core change here is the transformation of `run.logicalLocations` from a dictionary (which is keyed, generally, by the fully qualified name of a logical location) to an array of logical locations. Result locations now reference logical locations by a logical location index. This changes removes the necessity of resolving key name collisions for logical locations that differ only by type (a namespace that collides with the fully qualified name of a type being the classic example). In addition to making the core change, we have also authored a transformation that converts existing pre-release SARIF v2 files to the new design. We accomplish this by creating dictionaries, with value type comparison for keys, that are keyed by logical locations. This processing requires that any parent keys already exist in the array (so that a logical location can populate its parent logical location index, if any). In addition to the core functionality and any transformation of individual log files, result matching presents special complications. In a result matching scenario, the logical location index of a result is not relevant to its identify: only the contents of the logical location this index points to are relevant. Furthermore, when merging a baseline file (which can contain results that are exclusive to a single log file within the matching domain), logical location indices are subject to change and must be updated. For this scenario and at least one other, we use a visitor pattern to update indices. The reason is that locations are pervasive in the format and the visitor provides a convenient mechanism to put common location processing logical. This visitor uses puts additional pressure on the transformation logic, as it entails additional deserialization of the JSON. With more time/effort, we could have exhaustively updated all locations using the JParser/JObject/etc. API domain. Oh well. Finally, we must update the logical that transforms v1 to v2 and vice versa. Whew. If that was not already sufficiently intrusive, this work revealed some minor flaws in various converters (the ones that handle logical locations): AndroidStudio, FxCop and PREfast. This change is complex but valuable. Logical locations are now expressed as coherent objects in their table. In the main, I have preferred to leave `result.fullyQualifiedName` populated (in addition to `result.logicalLocationIndex`, to support easily looking up matching logical locations). * Add result.rank and ruleConfiguration.defaultRank (#1167) As we discussed offline with @fishoak, the design is good as it stands. The only change is that the default should be -1. I filed oasis-tcs/sarif-spec#303 for that, and put it on the agenda for TC #30. * Logical locations notes (#1184) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Logical locations notes (#1185) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * Incorporate "provenance" schema changes and fix package licenses (#1193) * Add autogenerated RuleConfiguration.cs missed from earlier commit. * Upgrade to NuGet.exe 4.9.2 to handle new license tag. * Remove unused 'Owners' element from build.props. * Add package Title. * Use packageLicenseExpression to specify package license. * Suppress NU5105 (about SemVer 2.0.0 strings) for "dotnet pack" packages. NuGet.exe still warns for ".nuspec" packages. * Incorporate latest "provenance" schema changes. * Address PR feedback. * External property files (#1194) * Update spec for externalPropertiesFile object. * Add external property files to schema. * Finish merge of provenance changes. * Update release notes. * Remove vertical whitespace. * PR feedback. Fix 'properties' to refer to an external file not an actual properties bag. * Remove code gen hint that makes external property files a property bag holder. * Introduce missing brace. Fix up code emit for 'properties' property that isn't a property bag. * Incorporate schema changes for versionControlDetails.mappedTo and rule.deprecatedIds (#1198) * Incorporate "versionControlDetails.mappedTo" schema change. * Incorporate "rule.deprecatedIds" schema change. * Revert updates to comprehensive.sarif (to allow transformer to continue to use this as test content). * Array scrub part 1: everything but anyOf-or-null properties. (#1201) NOTE: For explicitness, I added schema attributes even when they matched the JSON schema defaults, namely: `"minItems": 0` and `"uniqueItems": false`. * Fix v1->v2 hash transformation (#1203) CreateHash must be called to handle algorithm names that aren't in our translation table. Also updated a unit test to cover this case. * Integrate jschema 0.61.0 into SDK (#1204) * Merging arrays transformations back into 'develop' branch (#1236) * Fix up tests * Conversion to files array. WIP. Core SARIF component build complete except for SarifLogger tail. * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * Reduced files array build (#1191) * Sarif and Sarif.Converters now building * Files array (#1188) * Add fileIndex property to file object (#1186) * Fix up tests * PR feedback to improve schema comment * Logical locations notes (#1185) (#1187) * Respond to a small # of PR comments related to recent logical locations change. * Fix visibility on helper * Fix up v1 transformation with keys that collide * Preserve decorated name data * Rebaseline test for decorated name propagation * Respond to PR feedback. Update tests to close test holes. * Rebaseline updated test * Test key collision in annotated code locations. * Update baseline * DRY out converters to isolate shared code. * Restore essential change in schema that converts files dictionary to an array. * Simplify ShouldSerialize logic * Remove unused namespaces * Respond to PR feedback. * Respond to PR feedback * End-to-end build works. Now we can author core transformation and fix tests. (#1192) * Fix up merge from 'develop' branch. * Update supporting test code for processing checked in files. (#1202) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * Files array basic transform (#1205) * Update supporting test code for processing checked in files. * Update nested files test to contain single file. * WIP. Furhter progress * Fix up samples build * Fix up merge from basic transform branch * Mime type validation (#1206) * Fix up merge from basic transform branch * Fix up mime test * Start work on v1 <-> v2 transformation (#1208) * Restore TransformCommand and first (unaffected) unit test * Restore "minimal prerelease v2 to current v2" test. * estore "minimal v1 to current v2" test. * Restore remaining TransformCommand unit tests. * Uncomment v2 => v1 tests to observe failures. * Uncomment 'transform' command. * Restore MakeUrisAbsoluteVisitor tests (#1210) This change updates the visitor that expands URIs in the presence of `originalUriBaseIds`. Turns out there was technical debt here, because our tests provided `originalUriBaseIds` equivalents in the property bag (because we had no official SARIF slot for them). I did not notice this gap when we made the schema change to add `originalUriBaseIds`. * Get v2 -> v1 transform working with files array (#1211) Test failure count is down to 32; will be 28 when you merge your fix. There is not -- and never was -- a test case for fileLocations that use uriBaseId (never was one). I know for a fact that there is no code to support that case. You’ll see a comment to that effect in the code. I will take care of that next. Then I will move on to v1 -> v2 transform. As part of this change, the `SarifCurrentToVersionOneVisitorTests` are now based on the `RunTest` helper method from the base class `FileDiffingTests`. * Convert debug assert to exception to make test execution more deterministic (#1214) * Update insert optional data tests and update indices visitor. (#1212) * Update insert optional data tests and update indices visitor. * Delete speculatively needed file * Remove erroneous override of base visit method. * Rework summary comment on DeletesOutputsDirectoryOnClassInitializationFixture. * Update clang analyzer name. Flow converter log verification through JToken comparison. (#1213) * The multiool, core sarif, and validation test binaries now all pass (#1215) * The multiool, core sarif, and validation test binaries now all pass completely. * Remove unwanted assert that may fire during unit testing. * Merge from files-array * PR feedback. * PR feedback tweaks * Accept PR feedback from previous change. (#1216) Use LINQ IEnuemrable.Except in the unit test, which improves readability without compromising efficiency (because Except uses a Set to do its work in O(N) time). * Fix Sarif.Driver and Sarif.Functional tests. (#1217) * Fix Sarif.Driver and Sarif.Functional tests. * Restore test file * Fix Semmle tests and Fortify converter: all tests now pass. (#1218) * Sarif converters fixups (#1219) * Fix semmle tests and fority. * Final test fixups * Invoke appveyor for files-array branch.: (#1220) * Update SarifVersionOneToCurrentVisitor for run.files array (#1221) * Uncomment v1 -> v2 tests; 3/14 fail. * Move test data to locations expected by FileDiffingTests. * Fix up some IDE C#7 code cleanups. * Use FileDiffingTests helper. * Fix bug in FileDiffingTests that caused a test failure. * Remove default-valued argument from a call to RunTest. * Create basic files array Does not yet have fileIndex, parentIndex, or response file handling. * Revert incorrect change in FileDiffingTests. * Fix one unit test with spot fix to "expected" file. * Fix up some C#7 IDE warnings * Force update in FileDiffing tests to avoid deserialization errors from out of date "expected" files. * Fix missing "modified" flag sets in PreRelCompatTransformer. * Populate fileIndex in run.files array. * Fix unit test by fixing fileLocation creation. * Restore response file handling. * Populate fileIndex on fileLocations as appropriate. * Fix last test failure by reworking response file handling. * Feedback: Introduce transformer helper PopulatePropertyIfAbsent. * Feedback: Tighten platform-independent string compare. Also: - Reformat unit test lines. * Feedbakc: Revert FileDiffingTest change; downgrade affected test files to provoke transform * Basic rules transformation (except for v1 <-> v2 conversion) (#1223) * Basic rules transformation (except for v1 <-> v2 conversion) * Respond to very excellent PR feedback. * PR feedback * Add files array tests for nested files and uriBaseIds (#1226) * Add failing v1 -> v2 nested files test * Fix existing mis-handling of analysisTarget and resultFile. * Get nested files test case working. * Add failing v1 => v2 uriBaseId test. * Populate v2 uriBaseId. * Fix up expected v2 fileLocation.properties: test passes. * Enhance uriBaseIds test case. * Implement v2->v1 conversion for rules dictionary (#1228) * Notification rule index (#1229) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Notification rule index (#1230) * Add notification.ruleIndex and increase flatten messages testing * Notification message tests + add notification.ruleIndex to schema * Missed feedback from previous PR (#1231) * Implement v1->v2 conversion for rules dictionary (#1232) * Partial implementation * Get last test working. * Somebody missed checking in a generated file. * Schema changes from TC #30 (#1233) * Add source language, fix rank default. * Adjust rank minimum to accommoodate default. * Fix broken test. * Remove unnecessary None items from project file. * PR feedback * Files array results matching (#1234) * WIP preliminary results matching * Restore results matching for files array * Add back autogenerated (unused) namespace directive * Updated release notes for TC30 changes. (#1240) * Mention rules array change in release history. (#1243) * Baseline states (#1245) * Add 'updated' state to baselineState and rename 'existing' to 'unchanged' * Update prerelease transformer * Enable appveyor build + test. Correct version constant. * Update test. Respond to PR feedback. * Fix #1251 #1252 #1253 (#1254) * Fixes + test coverage + cleanup * Update SDK version * Update version in test assets * Fix multitool nuspec (#1256) * Revert unintentional change to BaselineState (#1262) The `develop` branch should match TC #30, but we inadvertently introduced a change from TC #31: replacing `BaselineState.Existing` with `.Unchanged` and `Updated`. I did not revert the entire change. Some things (like having AppVeyor build the `tc-31` branch instead of the defunct `files-array` branch, and some C# 7 updates to the `PrereleaseCompatibilityTransformer`) were good, and I kept them. Also: - Update the version to `2019-01-09` in preparation for merge to `master`. * Transfer Bogdan's point fix (analysisTarget handling) from master to develop (#1263) In preparation for merging `develop` to `master` for the publication of version 2019-01-09 (TC #30), we apply the recent changes in `master` to the `develop` branch. These changes fixed two bugs in the handling of `analysisTarget` in the v1-to-v2 converter (`SarifVersionOneToCurrentVisitor`). Now `develop` is completely up to date, and when we merge `develop` to `master`, we _should_ be able to simply take the "incoming" changes on all conflicting files. * Cherry-pick: v1 transformer analysis target region persistence fix. (#1238) * Mention NuGet publishing changes in release history. * Cherry pick: Don't emit v2 analysisTarget if there is no v1 resultFile. (#1247)

michaelcfanning added 2 commits January 13, 2019 19:57

Update insert optional data tests and update indices visitor.

e39fb52

Delete speculatively needed file

0e55d7c

michaelcfanning commented Jan 14, 2019

View reviewed changes

Remove erroneous override of base visit method.

a77802b

ghost reviewed Jan 15, 2019

View reviewed changes

Larry Golding added 2 commits January 14, 2019 17:24

Merge branch 'files-array' into tests-fixup

e8c2252

This picks up lgolding's fixes to the SarifCurrentToVersionOneVisitor and its tests.

Rework summary comment on DeletesOutputsDirectoryOnClassInitializatio…

1e0f22f

…nFixture.

ghost suggested changes Jan 15, 2019

View reviewed changes

ghost reviewed Jan 15, 2019

View reviewed changes

Merge branch 'files-array' into tests-fixup

e6768ff

michaelcfanning merged commit 03a36f9 into files-array Jan 15, 2019

ghost deleted the tests-fixup branch January 15, 2019 23:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update insert optional data tests and update indices visitor. #1212

Update insert optional data tests and update indices visitor. #1212

michaelcfanning commented Jan 14, 2019

michaelcfanning Jan 14, 2019 •

edited

Loading

ghost Jan 14, 2019

michaelcfanning Jan 14, 2019

ghost Jan 14, 2019

ghost Jan 15, 2019

ghost Jan 15, 2019 •

edited by ghost

Loading

michaelcfanning Jan 15, 2019

michaelcfanning Jan 14, 2019 •

edited

Loading

ghost Jan 14, 2019

michaelcfanning Jan 15, 2019

michaelcfanning commented Jan 14, 2019 •

edited by ghost

Loading

michaelcfanning Jan 14, 2019 •

edited by ghost

Loading

michaelcfanning Jan 14, 2019 •

edited

Loading

michaelcfanning commented Jan 14, 2019 •

edited by ghost

Loading

ghost commented Jan 14, 2019

ghost commented Jan 14, 2019

michaelcfanning commented Jan 14, 2019

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

michaelcfanning Jan 15, 2019

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost Jan 15, 2019 •

edited by ghost

Loading

ghost Jan 15, 2019

ghost Jan 15, 2019

ghost Jan 15, 2019

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

michaelcfanning Jan 15, 2019

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

michaelcfanning Jan 15, 2019

ghost left a comment

ghost commented Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

michaelcfanning Jan 15, 2019

michaelcfanning commented Jan 15, 2019

michaelcfanning commented Jan 15, 2019

		{
		public class FortifyFprConverterTestsFixture : DeletesOutputsDirectoryOnClassInitializationFixture { }


		public UpdateIndicesVisitor(IDictionary<string, int> fullyQualifiedLogicalNameToIndexMap, IDictionary<string, int> fileLocationKeyToIndexMap)
		public UpdateIndicesVisitor(IDictionary<string, int> fullyQualifiedLogicalNameToIndexMap, IDictionary<FileLocation, int> fileLocationToIndexMap)

Update insert optional data tests and update indices visitor. #1212

Update insert optional data tests and update indices visitor. #1212

Conversation

michaelcfanning commented Jan 14, 2019

michaelcfanning Jan 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jan 15, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning Jan 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning commented Jan 14, 2019 • edited by ghost Loading

michaelcfanning Jan 14, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

michaelcfanning Jan 14, 2019 • edited Loading

Choose a reason for hiding this comment

michaelcfanning commented Jan 14, 2019 • edited by ghost Loading

ghost commented Jan 14, 2019

ghost commented Jan 14, 2019

michaelcfanning commented Jan 14, 2019

ghost Jan 15, 2019 • edited by michaelcfanning Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jan 15, 2019 • edited by michaelcfanning Loading

Choose a reason for hiding this comment

ghost Jan 15, 2019 • edited by ghost Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jan 15, 2019 • edited by michaelcfanning Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost Jan 15, 2019 • edited by michaelcfanning Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

ghost commented Jan 15, 2019 • edited by michaelcfanning Loading

ghost Jan 15, 2019 • edited by michaelcfanning Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelcfanning commented Jan 15, 2019

michaelcfanning commented Jan 15, 2019

michaelcfanning Jan 14, 2019 •

edited

Loading

ghost Jan 15, 2019 •

edited by ghost

Loading

michaelcfanning Jan 14, 2019 •

edited

Loading

michaelcfanning commented Jan 14, 2019 •

edited by ghost

Loading

michaelcfanning Jan 14, 2019 •

edited by ghost

Loading

michaelcfanning Jan 14, 2019 •

edited

Loading

michaelcfanning commented Jan 14, 2019 •

edited by ghost

Loading

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost Jan 15, 2019 •

edited by ghost

Loading

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost commented Jan 15, 2019 •

edited by michaelcfanning

Loading

ghost Jan 15, 2019 •

edited by michaelcfanning

Loading