Skip to content

Feat/unittests#26

Open
thefirstfloor wants to merge 3 commits into
devfrom
feat/unittests
Open

Feat/unittests#26
thefirstfloor wants to merge 3 commits into
devfrom
feat/unittests

Conversation

@thefirstfloor

Copy link
Copy Markdown
Member

Looked at it, seems legit. Added docu so it's clear what is tested.
We might want to extend it with more tests.

@thefirstfloor thefirstfloor requested a review from eioe March 12, 2026 18:39
@thefirstfloor thefirstfloor self-assigned this Mar 12, 2026
@thefirstfloor thefirstfloor added the fix Fixes a known issue. label Mar 12, 2026
@eioe

eioe commented May 15, 2026

Copy link
Copy Markdown
Member

pls only commit relevant files. A lot is going on here, including adding new scripts of which is unclear what they are needed for (e.g., Rotator.cs). Very difficult to review.

The actual tests look either trivial or pretty opaque to me.
trivial:

var package = new EyeDataPackage();
package.eye = "left";
...
Assert.AreEqual("left", package.eye);

opaque:

var propertyInfo = typeof(EyeDataHandler).GetProperty("_currentSamples", BindingFlags.NonPublic | BindingFlags.Instance);
var currentSamples = (List<EyeDataPackage>)propertyInfo.GetValue(_handler);

No offense, but I cannot merge this with a clear conscience.

@eioe eioe closed this May 19, 2026
@thefirstfloor

Copy link
Copy Markdown
Member Author

@eioe What does closed mean?
I did not have any time to read and investigate into your comment. Please don't shut doors too enthousiasticly.

@thefirstfloor thefirstfloor reopened this May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Fixes a known issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants