menu-traversal-refactor #109

Closed
carlos wants to merge 24 commits from menu-traversal-refactor into master
Member

Introduction of the menu traversal tests for the core menu nodes

Introduction of the menu traversal tests for the core menu nodes
carlos added 9 commits 2024-10-07 22:37:17 +02:00
carlos added 1 commit 2024-10-07 23:03:47 +02:00
lash requested changes 2024-10-08 15:15:02 +02:00
@ -0,0 +31,4 @@
return sessionsData, err
}
func CreateTestCases(group DataGroup) []struct {
Owner

I prefer typedef instead of inline structs, the latter is hard to read.

should this be an extension of StepTest?

I prefer typedef instead of inline structs, the latter is hard to read. should this be an extension of StepTest?
Author
Member

Created a TestCase struct for generating the test cases: Commit : 810dd74e43.

Created a TestCase struct for generating the test cases: Commit : 810dd74e43936d999d2e6361c6b8a167c67a6317.
carlos marked this conversation as resolved
@ -0,0 +1,55 @@
package driver
Owner

please remove space in filename

please remove space in filename
Author
Member

Updated by commit: 3afe35b44f ,the setup driver functionality is now in the group driver.

Updated by commit: 3afe35b44f28cffbefe1a0f41b70a2c3ba1b4ccb ,the setup driver functionality is now in the group driver.
carlos marked this conversation as resolved
@ -0,0 +6,4 @@
"os"
)
type Step struct {
Owner

aren't these structs more or less the same as in the groupdriver.go

can't we just reuse the definitions?

aren't these structs more or less the same as in the groupdriver.go can't we just reuse the definitions?
Author
Member

Updated by commit: 3afe35b44f ,the setup driver functionality is now in the group driver.

Updated by commit: 3afe35b44f28cffbefe1a0f41b70a2c3ba1b4ccb ,the setup driver functionality is now in the group driver.
carlos marked this conversation as resolved
@ -0,0 +36,4 @@
return sessions
}
func FilterGroupsByName(groups []Group, name string) []Group {
Owner

Can we please use regex here, match start of string

Can we please use regex here, match start of string
carlos marked this conversation as resolved
engine/engine.go Outdated
@ -0,0 +1,111 @@
package engine
Owner

can we hide all of this under internal/testutil or something?

Also, I don't think we need a subdirectory engine/ for this, unless there is a specific point to it?

can we hide all of this under internal/testutil or something? Also, I don't think we need a subdirectory engine/ for this, unless there is a specific point to it?
Author
Member

This has now been moved to internal/testutil

This has now been moved to internal/testutil
Author
Member

This has now been moved to internal/testutil. Commit: 96aec1fd67

This has now been moved to internal/testutil. Commit: 96aec1fd670efbdf724596b6945a2ba152e83ff4
carlos marked this conversation as resolved
@ -0,0 +2,4 @@
package engine
const OnlineTestEnabled = false
Owner

I would just define the accountservice as a global directly here, instead of the detour of defining a variable

I would just define the accountservice as a global directly here, instead of the detour of defining a variable
@ -1 +1 @@
Confirm your new PIN:
Confirm your new PIN:
Owner

no visible change. stray non-printable char?

no visible change. stray non-printable char?
@ -18,3 +19,4 @@ INCMP enter_yob 4
INCMP enter_location 5
INCMP enter_offerings 6
INCMP view_profile 7
Owner

remove all blank lines please

remove all blank lines please
Author
Member

The blank line is required to be able to compile the .vis files,Without this,we would get an 'EOF' error.

The blank line is required to be able to compile the .vis files,Without this,we would get an 'EOF' error.
@ -0,0 +76,4 @@
t.Fatalf("Test case '%s' failed during Flush: %v", group.Name, err)
}
b := w.Bytes()
if !bytes.Equal(b, []byte(step.ExpectedContent)) {
Owner

Can you please implement the check as a function of the step struct (in reality, any struct that has ExpectedContent), and use regex for the match check instead?

The latter will enable us to have more flexible match criteria, as having literal checks everywhere will make it impossible to test dynamic data.

Can you please implement the check as a function of the step struct (in reality, any struct that has `ExpectedContent`), and use regex for the match check instead? The latter will enable us to have more flexible match criteria, as having literal checks everywhere will make it impossible to test dynamic data.
@ -0,0 +83,4 @@
}
}
// Adding a sleep after the test to wait for registration to complete the process
time.Sleep(5 * time.Second)
Owner

Only if run online right?

Can we make this a callback that could theoretically listen to an event channel instead? That means, abstract the wait for result (and in the case of offline test no wait).

Only if run online right? Can we make this a callback that could theoretically listen to an event channel instead? That means, abstract the wait for result (and in the case of offline test no wait).
Author
Member

@lash ,So the idea for this would be to setup an event channel in the TestEngine that would block the other test calls for 5 seconds ,if the online tag is defined.For reference and input: 96aec1fd67

@lash ,So the idea for this would be to setup an event channel in the TestEngine that would block the other test calls for 5 seconds ,if the online tag is defined.For reference and input: 96aec1fd670efbdf724596b6945a2ba152e83ff4
carlos added 6 commits 2024-10-09 15:47:35 +02:00
carlos added 2 commits 2024-10-09 21:22:42 +02:00
carlos added 1 commit 2024-10-10 08:13:26 +02:00
lash requested changes 2024-10-10 14:56:32 +02:00
@ -0,0 +27,4 @@
ctx = context.WithValue(ctx, "SessionId", sessionId)
pfp := path.Join(scriptDir, "pp.csv")
var eventChannel = make(chan bool)
Owner

My intuition tells me that having an abstraction of the account service TestAccountService that includes a method WaitForAccount would be easier to understand down the road.

Also, please see the comment about not using vars but instead setting that testaccountservice object explicitly as a global variable.

If this is unclear, I can try to explain better with an example if needed.

My intuition tells me that having an abstraction of the account service `TestAccountService` that includes a method `WaitForAccount` would be easier to understand down the road. Also, please see the comment about not using vars but instead setting that testaccountservice object explicitly as a global variable. If this is unclear, I can try to explain better with an example if needed.
Author
Member

@lash ,For clarity,you mind giving a simple example for using the testaccountservice globally?

@lash ,For clarity,you mind giving a simple example for using the testaccountservice globally?
carlos added 1 commit 2024-10-10 15:54:22 +02:00
carlos added 4 commits 2024-10-12 12:33:43 +02:00
Owner

obsoleted by #115

obsoleted by https://git.grassecon.net/urdt/ussd/pulls/115
lash closed this pull request 2024-10-18 15:28:36 +02:00

Pull request closed

Sign in to join this conversation.
No reviewers
No Milestone
No project
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: urdt/ussd#109
No description provided.