TrueCloudLab/contract-coverage-primer
21
0
Fork 2
Code Issues 4 Pull requests Projects Releases Packages Wiki Activity

Add a custom Invoke() function #10

Merged
fyrchik merged 1 commit from elebedeva/contract-coverage-primer:covertest/invoke into master 2024-09-04 19:51:22 +00:00
Conversation 0 Commits 1 Files changed 1 +117 -59
elebedeva commented 2023-08-08 14:31:26 +00:00
Member
Copy link

Close #4, #5, #6

Adding custom contract invoker with ability to produce coverage file from multiple testsuits.

Close #4, #5, #6 Adding custom contract invoker with ability to produce coverage file from multiple testsuits.
elebedeva added 2 commits 2023-08-08 14:31:27 +00:00 
Copy neotest.Invoke() and neotest.InvokeFail() functionality
and add modifications to collect coverage.

Signed-off-by: Ekaterina Lebedeva <ekaterina.lebedeva@yadro.com>
TestContract now use covertest invoker.

Signed-off-by: Ekaterina Lebedeva <ekaterina.lebedeva@yadro.com>
elebedeva force-pushed covertest/invoke from 902b9d5881 to 8921d5777c 2023-08-14 16:44:56 +00:00 Compare
fyrchik approved these changes 2023-08-15 14:23:39 +00:00
covertest/client.go
@ -0,0 +107,4 @@
}
ops, v, _ := TestInvoke(bc, tx) // ignore error to support failing transactions
tx.SystemFee = v.GasConsumed()
c.Methods[len(c.Methods)-1].Instructions = make([]InstrHash, len(ops))
fyrchik commented 2023-08-15 14:19:18 +00:00
Owner
Copy link

Could you explain the logic here? We take the last methogs in c.Methods slice an replace a set of instructions here:

  1. Why the last?
  2. Why replace and not append?
Could you explain the logic here? We take the last methogs in `c.Methods` slice an replace a set of instructions here: 1. Why the last? 2. Why replace and not append?
elebedeva commented 2023-08-16 14:56:41 +00:00
Author
Member
Copy link
  1. In Invoke() (or in InvokeFail()) we already appended current method (its name and nil for Instructions slice) to c.Methods. When we get a set of executed instructions from custom Run() we want to copy them to Instructions slice in current method (last one in c.Methods).
  2. If we append instructions to c.Methods[len(c.Methods)-1].Instructions there will be "tails" of allocated but unused memory. To avoid it we allocate just enough memory to copy the instructions slice there.
1. In `Invoke()` (or in `InvokeFail()`) we already appended current method (its name and nil for `Instructions` slice) to `c.Methods`. When we get a set of executed instructions from custom `Run()` we want to copy them to `Instructions` slice in current method (last one in `c.Methods`). 2. If we append instructions to `c.Methods[len(c.Methods)-1].Instructions` there will be "tails" of allocated but unused memory. To avoid it we allocate just enough memory to copy the instructions slice there.
fyrchik commented 2023-08-16 18:26:10 +00:00
Owner
Copy link

Ok, so am I getting it right:

  1. c.Methods corresponds to each Invoke, not to each method? So 3 invocations of the same method will produce 3 results.
  2. We still can test different methods in a single test?

Anyway, why do we create nil and access the last element, why not just append to c.Methods here?

Ok, so am I getting it right: 1. `c.Methods` corresponds to each `Invoke`, not to each method? So 3 invocations of the same method will produce 3 results. 2. We still _can_ test different methods in a single test? Anyway, why do we create nil and access the last element, why not just append to `c.Methods` here?
elebedeva commented 2023-08-17 13:36:02 +00:00
Author
Member
Copy link

Yes, that is correct.
Moved appending method to c.Methods here.

Yes, that is correct. Moved appending method to `c.Methods` here.
fyrchik marked this conversation as resolved
covertest/client.go Outdated
@ -0,0 +136,4 @@
return ops, ic.VM, err
}
type coverline struct {
fyrchik commented 2023-08-15 14:20:16 +00:00
Owner
Copy link

Golang already has https://godocs.io/testing#CoverBlock -- we might reuse it.

Golang already has https://godocs.io/testing#CoverBlock -- we might reuse it.
elebedeva commented 2023-08-21 11:46:08 +00:00
Author
Member
Copy link

Done

Done
fyrchik marked this conversation as resolved
covertest/client.go Outdated
@ -0,0 +178,4 @@
for _, method := range di.Methods {
maxLine := method.Range.End
for _, seqPoint := range method.SeqPoints {
if isValidDocument(seqPoint.Document, docs) && seqPoint.Opcode < int(maxLine) {
fyrchik commented 2023-08-15 14:20:54 +00:00
Owner
Copy link

What is the seqPoint.Opcode < int(maxLine) check here for?

What is the `seqPoint.Opcode < int(maxLine)` check here for?
elebedeva commented 2023-08-17 13:54:14 +00:00
Author
Member
Copy link

There is a problem with sequence points: some of them have strange opcodes which don't correspond to method opcodes. For example, in an example contract's method putNumber there is a SeqPoint with opcode 193 but the method don't have such opcode. This check solved the problem in the putNumber method but clearly don't solve the problem generally.

There is a problem with sequence points: some of them have strange opcodes which don't correspond to method opcodes. For example, in an example contract's method `putNumber` there is a SeqPoint with opcode 193 but the method don't have such opcode. This check solved the problem in the `putNumber` method but clearly don't solve the problem generally.
fyrchik commented 2023-08-21 17:23:20 +00:00
Owner
Copy link

Seems like a but in the compiler.

Seems like a but in the compiler.
fyrchik reviewed 2023-08-15 14:25:59 +00:00
covertest/client.go Outdated
@ -0,0 +205,4 @@
}
func countInstructions(cov []coverline, codes []InstrHash) {
for i := 0; i < len(cov); i++ {
fyrchik commented 2023-08-15 14:25:59 +00:00
Owner
Copy link

Why no for i := range cov?

Why no `for i := range cov`?
elebedeva commented 2023-08-16 14:58:38 +00:00
Author
Member
Copy link

Fixed

Fixed
fyrchik marked this conversation as resolved
elebedeva changed title from WIP: Add a custom `Invoke()` function to Add a custom `Invoke()` function 2023-08-15 14:42:07 +00:00
fyrchik reviewed 2023-08-16 18:38:03 +00:00
covertest/cover.go Outdated
@ -0,0 +118,4 @@
for _, info := range cov {
covered := 0
if info.IsCovered {
covered++
fyrchik commented 2023-08-16 18:37:39 +00:00
Owner
Copy link

I believe if we combine multiple expressions this way we will have both 0 and 1 for the same file + line, right?
They may interfere with each other, have you checked this is not the case?

I believe if we combine multiple expressions this way we will have both `0` and `1` for the same file + line, right? They may interfere with each other, have you checked this is not the case?
elebedeva commented 2023-08-21 13:02:49 +00:00
Author
Member
Copy link

Fixed.
Now the coverage output should be consistent.

Fixed. Now the coverage output should be consistent.
go.sum Outdated
@ -138,3 +158,4 @@
github.com/googleapis/gax-go/v2 v2.0.5/go.mod h1:DWXyrwAJ9X0FpwwEdw+IPEYBICEFu5mhpdKc/us6bOk=
github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc=
github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
fyrchik commented 2023-08-16 18:35:21 +00:00
Owner
Copy link

Do you do go mod tidy after dependency update?

Do you do `go mod tidy` after dependency update?
elebedeva commented 2023-08-17 14:05:04 +00:00
Author
Member
Copy link

Fixed

Fixed
fyrchik marked this conversation as resolved
tests/contract_test.go Outdated
@ -49,2 +49,4 @@
inv.InvokeFail(t, "Invalid key size", "putNumber", invalidKey, 42)
inv.InvokeFail(t, "Invalid key size", "getNumber", invalidKey)
// spew.Dump(inv.Methods)
inv.MakeCoverage(t, ctrDI, "contract.go", "c.out")
fyrchik commented 2023-08-16 18:34:54 +00:00
Owner
Copy link

What will happen if I execute tests multiple times after changing the code?
Will I get only the new coverage?

What will happen if I execute tests multiple times after changing the code? Will I get _only_ the new coverage?
elebedeva commented 2023-08-17 13:59:07 +00:00
Author
Member
Copy link

No, actually method writes to the end of the file, so new coverage will be added to existing one.

No, actually method writes to the end of the file, so new coverage will be added to existing one.
fyrchik commented 2023-08-21 07:13:53 +00:00
Owner
Copy link

It may not be desireable: when I modify code, I don't usually care what the old coverage was (an if I do, I can move it to a separate file).

It may not be desireable: when I modify code, I don't usually care what the old coverage was (an if I do, I can move it to a separate file).
elebedeva commented 2023-08-21 12:54:13 +00:00
Author
Member
Copy link

Fixed.
Now the new coverage replaces the old one after every test execution.

Fixed. Now the new coverage replaces the old one after every test execution.
fyrchik marked this conversation as resolved
elebedeva force-pushed covertest/invoke from fdb7293444 to 8e45fee116 2023-08-17 14:04:22 +00:00 Compare
elebedeva force-pushed covertest/invoke from 734468cfc9 to c51343019c 2023-08-21 12:25:27 +00:00 Compare
fyrchik approved these changes 2023-08-21 17:23:35 +00:00
fyrchik merged commit c51343019c into master 2023-08-21 17:23:46 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
fyrchik
Labels
Clear labels
  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/contract-coverage-primer#10
No description provided.
Delete branch "elebedeva/contract-coverage-primer:covertest/invoke"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?