search

Th0rstenf / Rocksniffer-to-Streamer.bot-extension

A set of streamerbot actions interacting with the Rocksmith RSSniffer
MIT License
6 stars 1 forks source link

Refactoring to prepare for addition of unit tests #46

Closed Th0rstenf closed 1 year ago

Th0rstenf commented 1 year ago

To keep up with rising complexity, automated SW tests could help as quality gate. If a suitable solution was implemented, a test driven approach for extensions would be encouraged.

Th0rstenf commented 1 year ago

@kozaka-tv Do you happen to know any good test frameworks for C#?

kozaka-tv commented 1 year ago

Not really.

https://www.lambdatest.com/blog/nunit-vs-xunit-vs-mstest/ https://saucelabs.com/resources/blog/nunit-vs-xunit-vs-mstest-with-examples

Maybe one of those?

kozaka-tv commented 1 year ago

But I would take NUnit Test I think.

Th0rstenf commented 1 year ago

Alright, then it will be NUnit, will still take some time to refactor.

In order to have cleaner public interfaces, i'd pack fetching and parsing the json to a nested class, and the whole evaluation to another. Then i'd suggest focussing on the second nest class in testing.

Th0rstenf commented 1 year ago

I've already spend a few hours on the refactoring itself, should be finished soonish (as soon work permits me to continue) I think this issue should be split into the refactoring itself, and then adding the test cases in a separate issue

Th0rstenf commented 1 year ago

Refactoring is progressing well. Functionality is split into several sub classes.

I'm just not sure how to work with the attributes. The main functionality is split in a way now that context can be set for testing pretty easy, but attributes are still encapsulated in the sub class.

Either I draw those out to the main class, and reference all of them in the subclass so they can be asserted in the test, OR I provide getter methods for all of them.

Tend to do the first one. Should group all those attributes into meaningfull structs to minimize the amount of conststructor parameters.

What are your thoughts @kozaka-tv ?

kozaka-tv commented 1 year ago

What I do not like, is the global variables. I think it would be better, to have 1-2 record classes for the data.

1) For config (ah...I forgot to do the refactoring regarding this in #52) 2) For variables like currentScene, sniffer status, etc ( 3) If needed... etc.)

And then, classes/methods could get those records as input. This way, we could call a method with any input, and test it.

https://www.baeldung.com/cs/global-variables

I do not know, that in SB we could use different files, like a real structured code, but most likely not. Only one file could be added and inside this, we have to do everything (this is the Program.cs). So we have to do everything in that single file. But test could be separated and, structured. Like one test the config read, the other the isRelevant Method or class, etc.

If, there would be an option, that somethhing assemble everything into one file, that would be great. Then we could split up Program.cs to smaller pieces, and then at release time (or during dev), we just assemble the Program.cs and then use it. That would be great. Idk that is this possible somehow....just thinking :)

Th0rstenf commented 1 year ago

They are not really global variables, they are still in CPHInline. It seems that NUnit offers methods to directly access those private attributes for testing as well, so this solves the problem most likely.

If you check the refactoring branch (still not finished though) it's already separating scene interactions, parsing the records from Sniffer and evaluating those records.

Am Mi., 22. März 2023 um 12:27 Uhr schrieb Kozaka @.***

:

What I do not like, is the global variables. I think it would be better, to have 1-2 record classes for the data.

  1. For config
  2. For variables like currentScene, sniffer status, etc ( 3) If needed... etc.)

And then, classes/methods could get those records as input. This way, we could call a method with any input, and test it.

https://www.baeldung.com/cs/global-variables

I do not know, that in SB we could use different files, like a real structured code, but most likely not. Only one file could be added and inside this, we have to do everything (this is the Program.cs). So we have to do everything in that single file. But test could be separated and, structured. Like one test the config read, the other the isRelevant Method or class, etc.

If, there would be an option, that somethhing assemble everything into one file, that would be great. Then we could split up Program.cs to smaller pieces, and then at release time (or during dev), we just assemble the Program.cs and then use it. That would be great. Idk that is this possible somehow....just thinking :)

— Reply to this email directly, view it on GitHub https://github.com/Th0rstenf/Rocksniffer-to-Streamer.bot-extension/issues/46#issuecomment-1479389830, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABJK4DPSTK3VZUTHHGOBXZDW5LOYLANCNFSM6AAAAAAVM73CNE . You are receiving this because you authored the thread.Message ID: <Th0rstenf/Rocksniffer-to-Streamer.bot-extension/issues/46/1479389830@ github.com>