search

robinsedlaczek / ForceFeedbackProgramming

18 stars 6 forks source link

find proj file(*.csproj, *.vbproj etc) or sln file(*.sln) and update … #70

Closed matt-inteltech closed 5 years ago

matt-inteltech commented 5 years ago

…config file's path

ralfw commented 5 years ago

This is looking reasonable :-)

What I'm missing, though, are automated tests.

You introduced functionality (finding the proj/sln folder starting from the folder where a .cs/.vb file is located; determining the full path to a config file based on the findings) - but that is intermingled with other functionality.

Such intermingling is unnecessary. There is no real dependency on anything specific to VS extensions in the functionality. Hence it's fairly easy to extract the new functionality into functions/classes of its own - and explicitly test it.

Please add tests for the major green streak in https://github.com/robinsedlaczek/ForceFeedbackProgramming/pull/70/files#diff-3d656f2d20ff518fa5a5cee9a154cd11 Use NUnit or xUnit as your test framework. And add the tests using a new special test project alongside the production code projects.

matt-inteltech commented 5 years ago

ok

matt-inteltech commented 5 years ago

I understand this test as;

  1. add a new special test project(ex: TestA) which has .sln and .csproj to relative path of FFP.
  2. add test project(ex: xUnitTest) by xUnit to FFP
  3. in test project(xUnitTest), open automatically or manually TestA and run new functionality that i made.

Is this right?

ralfw commented 5 years ago

The whole extension is located inside a .sln file. To that .sln file just add a single test project (DLL) which uses NUnit or xUnit. And the test in that project should target the new functionality you added for this issue. Of course in a meaningful way.

matt-inteltech commented 5 years ago

I added new functionality and xUnitTest

matt-inteltech commented 5 years ago

Give me next work.

matt-inteltech commented 5 years ago

can i add 2 hours for xUnitTest?

ralfw commented 5 years ago

So far I haven't seen any meaningful unit tests, sorry. Before we talk hours, please answer to my review questions by adding relevant tests.

matt-inteltech commented 5 years ago

ok

ralfw commented 5 years ago

Please do not (!) add a solution for every test! Just 1 solution for all test is sufficient. Even a single class for all tests of a single class is sufficient.

matt-inteltech commented 5 years ago

image

I updated test.

ralfw commented 5 years ago

Sorry, this is insufficient.

  1. What you're doing is checking for files. That's not testing the functionality you wrote.
  2. There are no distinct test cases. What you're doing is all happening within a single test function.

Your system-under-test is the Global class. For its functionality you need to write tests.

matt-inteltech commented 5 years ago

can you give me good feedback in upwork? After you read this comment, I will delete.

robinsedlaczek commented 5 years ago

I think it's better for us all to close this PR now. :)