Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Get tests running & give pseudocode in tests like Java assessment #109

Merged
merged 11 commits into from
Oct 8, 2024
Merged

Get tests running & give pseudocode in tests like Java assessment #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
27a3213
tests running
twinlensreflex Jul 16, 2024
4bf2589
Alterations
rosebbc Jul 17, 2024
572c3d2
Update election-api-scala/README.md
twinlensreflex Oct 7, 2024
650a6bd
Code review suggestions (#9)
twinlensreflex Oct 7, 2024
ad333f0
Revert "Code review suggestions (#9)"
twinlensreflex Oct 7, 2024
516ade6
attempt to revert README where change needs to be applied everywhere
twinlensreflex Oct 7, 2024
f29a7a1
Merge branch 'main' of github.com:bbc/rms-software-engineering-techni…
twinlensreflex Oct 7, 2024
1562308
effectively revert changes to tasks file
twinlensreflex Oct 7, 2024
7d6780b
Revert "effectively revert changes to tasks file"
twinlensreflex Oct 7, 2024
07a4ef4
revert scala tasks.md
twinlensreflex Oct 8, 2024
4af33ea
update TODO comment
twinlensreflex Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions election-api-scala/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ During your assessment we will ask you to work though the task in `tasks.md` wit

`sbt run`

You may get a port error when you try to run this. If so, run the following command:

`export PLAY_HTTP_PORT=9001`

### To Test

`sbt test`
3 changes: 1 addition & 2 deletions election-api-scala/app/controllers/ResultsController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class ResultsController @Inject()(val controllerComponents: ControllerComponents
}

def getScoreboard: Action[AnyContent] = Action {

Ok(Json.toJson(Scoreboard(0)))
Ok(Json.toJson(None))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I recall from Python & Java, this is "supposed" to fail to begin with because Scoreboard isn't adequately implemented/integrated yet. I'm not sure that's the right way but it is consistent

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've had discussions around this in our team. The original implementation has declared as a parameter. Bizarrely this isn't mentioned anywhere in the instructions which is confusing. Since this is written using the Play framework if we remove the param on the case class the serialization breaks - it asks for a parameter. We have to make a choice about what parameter we want to put in and we decided to put winner because it's referred to in the instructions.

}
}
5 changes: 3 additions & 2 deletions election-api-scala/app/model/PartyResult.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ case class ConstituencyResult(id: Int, name: String, seqNo: Int, partyResults: S

case class ApiResponse (error: String, message: String)


case class Scoreboard(declared:Int)
// TODO:
// implement this in any way you wish
case class Scoreboard(winner: String)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want winner parameter here? Feels like prejudicing the approach taken...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could potentially remove the comment here or change it to something like this:

// TODO: 
// implement this in any way you wish

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like that - the other languages don't lead the code approach like this, it's just the tests that imply a winner field needs to be grabbed somehow.

5 changes: 2 additions & 3 deletions election-api-scala/build.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ version := "1.0-SNAPSHOT"

lazy val root = (project in file(".")).enablePlugins(PlayScala)

scalaVersion := "2.13.6"
scalaVersion := "2.13.14"

libraryDependencies += guice
libraryDependencies += "org.scalatestplus.play" %% "scalatestplus-play" % "5.0.0" % Test

libraryDependencies += "org.scalatestplus.play" %% "scalatestplus-play" % "7.0.1" % Test
2 changes: 1 addition & 1 deletion election-api-scala/project/build.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
sbt.version=1.5.2
sbt.version=1.10.1
4 changes: 2 additions & 2 deletions election-api-scala/project/plugins.sbt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
addSbtPlugin("com.typesafe.play" % "sbt-plugin" % "2.8.8")
addSbtPlugin("org.foundweekends.giter8" % "sbt-giter8-scaffold" % "0.11.0")
addSbtPlugin("org.playframework" % "sbt-plugin" % "3.0.4")
addSbtPlugin("org.foundweekends.giter8" % "sbt-giter8-scaffold" % "0.16.2")
50 changes: 19 additions & 31 deletions election-api-scala/test/controllers/ResultsControllerSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package controllers

import akka.stream.Materializer
import model.Scoreboard
import org.scalatestplus.play._
import org.scalatestplus.play.guice._
Expand All @@ -10,7 +9,6 @@ import play.api.libs.json.{Json, OFormat}
import service.MapResultService

class ResultsControllerSpec extends PlaySpec with GuiceOneAppPerTest with Injecting {
implicit lazy val materializer: Materializer = app.materializer
//TODO: You will need the json format for any classes used within the Scoreboard here
implicit val scoreboardJson: OFormat[Scoreboard] = Json.format[Scoreboard]

Expand All @@ -21,47 +19,37 @@ class ResultsControllerSpec extends PlaySpec with GuiceOneAppPerTest with Inject

"Test first 5 results" in {
val scoreboard = runXResults(5)
scoreboard match {
case Some(sc) => matchScoreboard(scoreboard = sc, LabSeats = 4, LDSeats = 1)
case None => fail("Didn't return a scoreboard")
}
scoreboard must not be empty
// LAB = 4
// LD = 1
// winner = noone
}

"First 100 results" in {
val scoreboard = runXResults(100)
scoreboard match {
case Some(sc) => matchScoreboard(scoreboard = sc, LabSeats = 56, LDSeats = 12, ConSeats = 31)
case None => fail("Didn't return a scoreboard")
}
scoreboard must not be empty
// LD == 12
// LAB == 56
// CON == 31
// winner = noone
}

"First 554 results" in {
val scoreboard = runXResults(554)
scoreboard match {
case Some(sc) => matchScoreboard(scoreboard = sc, LabSeats = 325, LDSeats = 52, ConSeats = 167, winner = Some("LAB"))
case None => fail("Didn't return a scoreboard")
}
scoreboard must not be empty
// LD == 52
// LAB = 325
// CON = 167
// winner = LAB
}

"All results" in {
val scoreboard = runXResults(650)
scoreboard match {
case Some(sc) => matchScoreboard(scoreboard = sc, LabSeats = 349, LDSeats = 62, ConSeats = 210, winner = Some("LAB"))
case None => fail("Didn't return a scoreboard")
}
}

def matchScoreboard(scoreboard: Scoreboard, LabSeats: Int = 0,
LDSeats: Int = 0, ConSeats: Int = 0, winner: Option[String] = None) {
var ld, lab, con = 0

//TODO: set the seats by party

ld mustEqual LDSeats
lab mustEqual LabSeats
con mustEqual ConSeats
// something mustEqual winner
fail("Implement me")
scoreboard must not be empty
// LD == 62
// LAB == 349
// CON == 210
// winner = LAB
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on having comments stating assertions needed & letting the candidate implement (as in other languages)


def runXResults(number: Int): Option[Scoreboard] = {
Expand Down