AC-FuSa-Tools / nav

navigate kernel database
GNU General Public License v2.0
3 stars 4 forks source link

Testing refactor and coverage #19

Closed odra closed 1 year ago

odra commented 1 year ago

Some functions require refactoring for testing and I would rather adding tests as we enhance for the sake of PR context.

Changes

Patch file can be found at https://gist.github.com/odra/ac37c24a4b599aee7712c4b9c28a896a

alessandrocarminati commented 1 year ago

I noticed that you added a lot of dependencies for the test (more I expected), but I believe I can manage them. Although I feel uneasy when numerous dependencies are added, I understand the importance of writing tests in the way you did. If you don't mind, could you please add a patch to this pull request to enhance its readability?

From eba516b7bdf29648e89889788fe242ab70c5053d Mon Sep 17 00:00:00 2001
From: Alessandro Carminati <alessandro.carminati@gmail.com>
Date: Tue, 21 Mar 2023 12:51:16 +0100
Subject: [PATCH] use const values in opt2num tests in place of numbers

Signed-off-by: Alessandro Carminati <alessandro.carminati@gmail.com>
---
 nav.go      | 10 +++++-----
 nav_test.go | 10 +++++-----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/nav.go b/nav.go
index ac12722..be4d635 100644
--- a/nav.go
+++ b/nav.go
@@ -17,7 +17,7 @@ import (
 )

 const (
-   dummyOutput int = iota
+   invalidOutput int = iota
    graphOnly
    jsonOutputPlain
    jsonOutputB64
@@ -47,10 +47,10 @@ var fmtDotNodeHighlightWoSymb = "\"%[1]s\" [shape=record style=\"rounded,filled,

 func opt2num(s string) int {
    var opt = map[string]int{
-       "graphOnly":       1,
-       "jsonOutputPlain": 2,
-       "jsonOutputB64":   3,
-       "jsonOutputGZB64": 4,
+       "graphOnly":       graphOnly,
+       "jsonOutputPlain": jsonOutputPlain,
+       "jsonOutputB64":   jsonOutputB64,
+       "jsonOutputGZB64": jsonOutputGZB64,
    }
    val, ok := opt[s]
    if !ok {
diff --git a/nav_test.go b/nav_test.go
index a501090..9380f71 100644
--- a/nav_test.go
+++ b/nav_test.go
@@ -13,25 +13,25 @@ var _ = Describe("Nav Tests", func() {
    Describe("opt2num", func() {
        When("Using a valid options key", func() {
            It("Should return the correct value for graphOnly", func() {
-               Expect(opt2num("graphOnly")).To(Equal(1))
+               Expect(opt2num("graphOnly")).To(Equal(graphOnly))
            })

            It("Should return the correct value for jsonOutputPlain", func() {
-               Expect(opt2num("jsonOutputPlain")).To(Equal(2))
+               Expect(opt2num("jsonOutputPlain")).To(Equal(jsonOutputPlain))
            })

            It("Should return the correct value for jsonOutputB64", func() {
-               Expect(opt2num("jsonOutputB64")).To(Equal(3))
+               Expect(opt2num("jsonOutputB64")).To(Equal(jsonOutputB64))
            })

            It("Should return the correct value for jsonOutputGZB64", func() {
-               Expect(opt2num("jsonOutputGZB64")).To(Equal(4))
+               Expect(opt2num("jsonOutputGZB64")).To(Equal(jsonOutputGZB64))
            })
        })

        When("Using an invalid options key", func() {
            It("Should return 0", func() {
-               Expect(opt2num("invalidKey")).To(Equal(0))
+               Expect(opt2num("invalidKey")).To(Equal(invalidOutput))
            })
        })
    })
-- 
2.30.2
odra commented 1 year ago

Sure, np. Let me rebase it first.

odra commented 1 year ago

@alessandrocarminati I've added a link to gist url which contains the actual patch diff.

alessandrocarminati commented 1 year ago

You didn't add the patch I suggested, do you want me to add it after having your work merged?

alessandrocarminati commented 1 year ago

Looks good. Thanks!