-
Notifications
You must be signed in to change notification settings - Fork 7
Sh/#646 rewrite groovy test to scala test #726
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Sh/#646 rewrite groovy test to scala test #726
Conversation
sebastian-peter
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor remarks from my side that are (hopefully) easy to fix. Good work!
src/test/scala/edu/ie3/simona/model/participant/PvModelSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/participant/PvModelSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/participant/PvModelSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/participant/PvModelSpec.scala
Outdated
Show resolved
Hide resolved
src/test/scala/edu/ie3/simona/model/participant/PvModelSpec.scala
Outdated
Show resolved
Hide resolved
|
I noticed that the constructor for PvInput is no longer working here. I looked it up and think it is because of the input parameter "EmInput" which is missing. Is that something that was only recently added to the PvInput class? |
|
Yes, this change was introduced with #751. Before the em was told which entities he controlls. We changed this, so now the entities, here the PV, needs to get an em (or none, if the pv is not em controlled). However, it should work if you add |
danielfeismann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work, I guess we're already done! I only miss a changelog entry and since all tests are transfered you could delete PvModelTest.groovy
…la-test # Conflicts: # CHANGELOG.md
danielfeismann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good to me, thanks for your work done here.
Here is the finished version of PVModelTest.scala.
This PR is related to Issue #646