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
Change tests for event rules #88
Conversation
gcgbarbosa
commented
Jun 19, 2020
•
edited
edited
- Now all event tests extend EventSpec
- Helper fixtures moved to EventSpec
@gcgbarbosa why doesn't your PR has a codecov report? |
@marcovzla it does. The report was generated. The question is: why codecov bot sometimes come and sometimes does not come :( You can find the report here: |
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.
I left a comment in the code. Also, can you look into the problem with the codecov bot?
Thanks!
|
||
def getJsonDocument(id: String): String = { | ||
val json = Map( | ||
"1" -> """{"id":"48fb577b-f5ba-4e16-864f-f8ba20ba9cfa","metadata":[],"sentences":[{"numTokens":8,"fields":[{"$type":"ai.lum.odinson.TokensField","name":"raw","tokens":["The","consumption","of","gummy","bears","and","donuts","."],"store":true},{"$type":"ai.lum.odinson.TokensField","name":"word","tokens":["The","consumption","of","gummy","bears","and","donuts","."]},{"$type":"ai.lum.odinson.TokensField","name":"tag","tokens":["DT","NN","IN","NN","NNS","CC","NNS","."]},{"$type":"ai.lum.odinson.TokensField","name":"lemma","tokens":["the","consumption","of","gummy","bear","and","donut","."]},{"$type":"ai.lum.odinson.TokensField","name":"entity","tokens":["O","O","O","B-dessert","I-dessert","O","B-dessert","O"]},{"$type":"ai.lum.odinson.TokensField","name":"chunk","tokens":["B-NP","I-NP","B-PP","B-NP","I-NP","O","B-NP","O"]},{"$type":"ai.lum.odinson.GraphField","name":"dependencies","edges":[[1,0,"det"],[1,4,"nmod_of"],[1,7,"punct"],[4,2,"case"],[4,3,"compound"],[4,5,"cc"],[4,6,"conj"]],"roots":[1]}]}]}""", |
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.
Having all the docs in one place is great, but can you give them names that make it easier to know which one we're grabbing? Maybe add as a comment a mapping from name to contents, i.e., the actual sentences in the doc.
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.
Do you think using the id is enough?
I was planning to have a TAG svg of all documents used for test somewhere in a readme.
Meanwhile the only thing I could think would be informative would be the document id.jk
I noticed that the report says |
Yeah, I just noticed the same thing. Whenever codecove detecs the CI status it sends the bot. It is not detecting our CI status for some reason. |
I found a few people saying that installing the code-cov github plugin would solve the problem, so I went ahead and asked for permition to install the plugin. Similar issue: |
I **love** the idea of a TAG svg of all the docs!!!!! love love love it (at
least all docs that are short enough to be feasible!)
I think an id could be something like "ninjaTurtles" for the ninja turtles
sentence, or "gummyBears", etc... it doesn't need to be exhaustive, but
something human-readable that we can use as a reference
…On Mon, Jun 22, 2020 at 7:18 PM George C. G. Barbosa < ***@***.***> wrote:
*External Email*
***@***.**** commented on this pull request.
------------------------------
In core/src/test/scala/ai/lum/odinson/events/EventSpec.scala
<#88 (comment)>:
> + val matchedForThisRole = groupedMatched(desiredRole)
+ desired should have size matchedForThisRole.size
+ for (d <- desired) {
+ matchedForThisRole should contain (d)
+ }
+ // there shouldn't be any found arguments that we didn't want
+ val unwantedArgs = groupedMatched.keySet.diff(groupedDesired.keySet)
+ unwantedArgs shouldBe empty
+ }
+ }
+
+ def createArgument(name: String, start: Int, end: Int): Argument = Argument(name, start, end)
+
+ def getJsonDocument(id: String): String = {
+ val json = Map(
+ "1" -> """{"id":"48fb577b-f5ba-4e16-864f-f8ba20ba9cfa","metadata":[],"sentences":[{"numTokens":8,"fields":[{"$type":"ai.lum.odinson.TokensField","name":"raw","tokens":["The","consumption","of","gummy","bears","and","donuts","."],"store":true},{"$type":"ai.lum.odinson.TokensField","name":"word","tokens":["The","consumption","of","gummy","bears","and","donuts","."]},{"$type":"ai.lum.odinson.TokensField","name":"tag","tokens":["DT","NN","IN","NN","NNS","CC","NNS","."]},{"$type":"ai.lum.odinson.TokensField","name":"lemma","tokens":["the","consumption","of","gummy","bear","and","donut","."]},{"$type":"ai.lum.odinson.TokensField","name":"entity","tokens":["O","O","O","B-dessert","I-dessert","O","B-dessert","O"]},{"$type":"ai.lum.odinson.TokensField","name":"chunk","tokens":["B-NP","I-NP","B-PP","B-NP","I-NP","O","B-NP","O"]},{"$type":"ai.lum.odinson.GraphField","name":"dependencies","edges":[[1,0,"det"],[1,4,"nmod_of"],[1,7,"punct"],[4,2,"case"],[4,3,"compound"],[4,5,"cc"],[4,6,"conj"]],"roots":[1]}]}]}""",
Do you think using the id is enough?
I was planning to have a TAG svg of all documents used for test somewhere
in a readme.
Meanwhile the only thing I could think would be informative would be the
document id.jk
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#88 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABJCPCOZ4G43SK5AM6EJT5TRYAGIBANCNFSM4OC73DZQ>
.
|
Codecov Report
@@ Coverage Diff @@
## master #88 +/- ##
==========================================
- Coverage 56.51% 56.34% -0.18%
==========================================
Files 71 71
Lines 2870 2870
Branches 228 228
==========================================
- Hits 1622 1617 -5
- Misses 1248 1253 +5
Continue to review full report at Codecov.
|
@gcgbarbosa There is a conflict. I just merged #94 so that could be it. |
@marcovzla done. |