Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • Oct 15 14:37
    karelz review_requested #43116
  • Oct 15 14:37
    karelz labeled #43116
  • Oct 15 14:37
    karelz ready_for_review #43116
  • Oct 15 14:29

    mmitche on 3.1

    remove rhel6 (#43115) (compare)

  • Oct 15 14:28
    mmitche closed #43115
  • Oct 15 13:22
    karelz milestoned #43116
  • Oct 15 13:21
    karelz labeled #43116
  • Oct 15 13:21
    karelz assigned #43116
  • Oct 15 13:21
    karelz edited #43116
  • Oct 15 13:17
    ZJ13790777554 starred dotnet/corefx
  • Oct 15 13:01
    ManickaP edited #43116
  • Oct 15 12:47
    ManickaP synchronize #43116
  • Oct 15 12:45
    ManickaP opened #43116
  • Oct 15 10:19
    lumiyuki starred dotnet/corefx
  • Oct 15 08:59
    lumiyuki starred dotnet/corefx
  • Oct 15 00:37
    vseanreesermsft opened #43115
  • Oct 14 20:50

    jonfortescue on 3.1

    Switch to 1ES servicing pools o… Merge pull request #43109 from … (compare)

  • Oct 14 20:50
    jonfortescue closed #43109
  • Oct 14 16:24
    ruthietut starred dotnet/corefx
  • Oct 13 17:20

    vseanreesermsft on 3.1

    Merge in 'release/3.1' changes Merge in 'release/3.1' changes Merged PR 18168: [internal/rele… and 2 more (compare)

CyrusNajmabadi
@CyrusNajmabadi
git config --global core.autocrlf false
Now your git client won't change line endings
Mohammad Hamdy Ghanem
@VBAndCs

Now your git client won't change line endings

Nice. Thanks.

The implication is that you're hashing one of your code files. Why?

I decided to use hashing in test methods that test the generated code. Input line terminator doesn't matter, but the output must have the same hash that the one I am comparing to.

I am using a vb-like code to generate a record class. I am using Roslyn to parse my syntax. So, the input is code, and the output is also code.
Mohammad Hamdy Ghanem
@VBAndCs
I felt tired of pasting the whole long class in each test method, so, tried the hashing.
Assert.AreEqual(GetHash(result.Output), "BC7C020F61E28086AD90B7E27DA8B21E") seems nice :)
Stephen A. Imhoff
@Clockwork-Muse
... as opposed to comparing file contents, or a syntax tree diff? Which could even be used to tell you where the difference in the file is. If the hash is different now you have a mystery.
Also, updating the hash becomes a self-fulfilling prophecy for these tests: "I changed the generation, so the hash changes, and the test outputs the new hash..."

GitHub didn't care. This relates to your git client settings.

For this sort of situation I wish you could explicitly set certain files to be a specific file ending. Especially since I think GitHub may have mucked with them on top of things, too.

CyrusNajmabadi
@CyrusNajmabadi
I decided to use hashing in test methods that test the generated code.
I guess the question is: why?
Why not actually just validate the contents?
Assert.AreEqual(GetHash(result.Output), "BC7C020F61E28086AD90B7E27DA8B21E")
This seems completely not understandable. I have no way to know what the actual expected outcome is. So if somethign breaks all i get is a message saying "XYZ" != "ABC". that doesn't tell me why things are broken or what i would need to do to get a correct hash again.
Mohammad Hamdy Ghanem
@VBAndCs
Think of this as an Integral Test, where I check that the whole generator is working without exception, and the the output has the expected fingerprint.
<TestMethod>
    Public Sub NameValue()
        Dim TestRecord = <![CDATA[Public Record NameValue(Name ="", Value = 0.0)]]>.Value
        Dim result = GetGeneratedOutput(TestSourceCode, TestRecord)
        For Each diag In result.Diagnostics
            Assert.AreNotEqual(diag.Id, "BC42502", diag.ToString())
        Next
        Assert.AreEqual(GetHash(result.Output), "BC7C020F61E28086AD90B7E27DA8B21E")
    End Sub

    <TestMethod>
CyrusNajmabadi
@CyrusNajmabadi
yes. i get that. i'm saying: the test isn't meaningful. because if i get a different hash i have no way to ascertain if that's a good change in behavior, or an unwanted one.
the 'previous' result means nothing to me. no one can look at BC7C020F61E28086AD90B7E27DA8B21E and understand waht it means.
and if the new hash was different, having hte unit test say "you expected BC7C020F61E28086AD90B7E27DA8B21E, but got <something else>" isn't actionable.
Mohammad Hamdy Ghanem
@VBAndCs
I have another unit tests to test critical parts:

    <TestMethod>
    Public Sub WriteNameValue()
        Dim code = <![CDATA[(Name ="", Value = 0.0)]]>.Value

        Dim expected = <![CDATA[   <Key>
   <DefaultValue("1= #1/1/0001#")>
   Public Shadows Property [Date] As Date

   <Key>
   <DefaultValue("1=__QUOTE____QUOTE__")>
   Public Property [Name] As String

   <Key>
   <DefaultValue("1= 0.0")>
   Public Property [Value] As Double


]]>.Value

        Dim properties As New List(Of PropertyInfo)
        Dim result = WriteProperties(code, properties, "Inherits Test").Replace(vbCrLf, vbLf)
        Assert.AreEqual(result, expected)


        expected = <![CDATA[    Public Sub New(
                Optional [date] As Date = #1/1/0001#,
                Optional [name] As String ="",
                Optional [value] As Double = 0.0
            )

        Me.Date = [date]
        Me.Name = [name]
        Me.Value = [value]
    End Sub

]]>.Value

        Dim sb As New StringBuilder
        RecordParser.WriteConstructor(properties, sb)
        result = sb.Replace(vbCrLf, vbLf).ToString()
        Assert.AreEqual(result, expected)


        expected = <![CDATA[    Public Function [With](
                Optional [date] As Date? = Nothing,
                Optional [name] As [Optional](Of String) = Nothing,
                Optional [value] As Double? = Nothing
            ) As NameValue

        Return  New NameValue(
            If ([date].HasValue, [date].Value, Me.Date),
            If ([name].HasValue, [name].Value, Me.Name),
            If ([value].HasValue, [value].Value, Me.Value)
        )
    End Function

]]>.Value
        sb.Clear()
        RecordParser.WriteWith("NameValue", "", properties, sb)
        result = sb.Replace(vbCrLf, vbLf).ToString()
        Assert.AreEqual(result, expected)

    End Sub
CyrusNajmabadi
@CyrusNajmabadi
none of that addresses teh point i made about your test :)
if it fails. it's not actionable.
Mohammad Hamdy Ghanem
@VBAndCs
@Clockwork-Muse Any change in the generator that changes the output, will need to change all the tests expected values, using hashing or plain strings.
CyrusNajmabadi
@CyrusNajmabadi
Any change in the generator that changes the output, will need to change all the tests expected values
taht's not necessarily true
it will be true if you're doing an exact check of contents (which is up to you)
you could, for example, do a whitespace-insensitive check, if you want your tests to effectively say: i don't care if whitespace changes
the hashing approach is somewhat the worst of all worlds
Stephen A. Imhoff
@Clockwork-Muse

you could, for example, do a whitespace-insensitive check, if you want your tests to effectively say: i don't care if whitespace changes

Right, this

CyrusNajmabadi
@CyrusNajmabadi
it always changes whenever the output cahnges (no matter how slight). but it gives the least amoutn of information about what changed.
it soundsl ike you want hte opposite. you want slight changes in your output to to not cause assertion failures.
Stephen A. Imhoff
@Clockwork-Muse
Which is why I brought up the point about syntax tree checking, because often for generators you don't care what it looks like.
Mohammad Hamdy Ghanem
@VBAndCs

if it fails. it's not actionable.

If NameValue integral test fails, I will run WriteNameValue and see what is going on.

CyrusNajmabadi
@CyrusNajmabadi
I will run WriteNameValue and see what is going on.
why don't you just always run this then :)
they're tests :)
Mohammad Hamdy Ghanem
@VBAndCs
In fact I need these tests as a debugging entry points rather than tests.
CyrusNajmabadi
@CyrusNajmabadi
just have each test call your 'invariant checking test helper'
if you already have a test helper which can tell you precisely what is different... then that's what your tests should use. you shouldn't need a manual step fo going "oh, the hash changed, let me manually run this helper to find out why"
the tests can all run without manual intervention
Mohammad Hamdy Ghanem
@VBAndCs
I ran them all after every change. I will debug into some of them to see why things fail.
CyrusNajmabadi
@CyrusNajmabadi
...
i mean, you're proving the point :)
Mohammad Hamdy Ghanem
@VBAndCs
This helps me while updating the generator.
CyrusNajmabadi
@CyrusNajmabadi
you're picking a lossy testing strategy that requires you to have to debug in to figure out what went wrong when it fails.
let's step back.
i get yoru goal here. you want to be able to updat ethe generator, but not have to deal with lots of fallout when somethign changes
however the hashing approach does not provide that. indeed, it's worse. because any slight changes in input produces a new hash.
Mohammad Hamdy Ghanem
@VBAndCs
yes