OVS unit tests. JJB yaml file. #1

capsali · 2017-02-01T14:43:35Z

No description provided.

ociuhandu · 2017-02-01T15:00:31Z

General notes:

function naming: what's the point of naming most functions like "Start-XXX"? I think those have to be renamed to actually show what the functions do.
2.config/environment/build variables should be stored in a "config.ps1" file. Also, the declared functions in a "library/utils" file. Those get imported and then you have the "logic" in the main file, without all the functions declarations.
instead of "cd $folder" we rely on pushd $folder + popd once the task in that folder is finished. This way we avoid issues with the current location. And we don't use (unless really justified) relative paths. Relative path = $base_path_in_variable + $relative_path

ociuhandu · 2017-02-01T15:02:47Z

scripts/unit-tests/linux/ovs-git.sh

+    head -1 $WORKSPACE/gitlog.txt | tee $WORKSPACE/lastrancommit.txt
+}
+
+function Start-CommitJob {


I guess that this function does is starting a job for checking a commit, so the naming should reflect that, it's not "starting a commit job" - it won't commit anything.

Renamed the functions to reflect what they actually do

ociuhandu · 2017-02-01T15:04:09Z

scripts/unit-tests/linux/ovs-git.sh

+    done < $WORKSPACE/gitlog.txt
+}
+
+export WORKSPACE="/var/lib/jenkins/jobs/master-job/workspace"


You never hardcode a workspace that's inside the jenkins-managed workspaces. If you need a common workspace to be shared by multiple jobs, you use a dedicated folder created for that.

this was a path that was used when testing and forgot to remove it prior to PR

ociuhandu · 2017-02-01T15:08:20Z

scripts/unit-tests/linux/ovs-git.sh

+}
+
+function Get-GitLog {
+    currentcommit=`cat $WORKSPACE/lastrancommit.txt`


please use powershell cmd-lets, not aliases that are "linux-like"

ociuhandu · 2017-02-01T15:09:02Z

scripts/unit-tests/linux/ovs-git.sh

+    while read line
+    do
+        echo "Starting job for commitID $line"
+        curl -X POST "http://$jenk_user:$jenk_api@10.20.1.3:8080/job/ovs-build-job/buildWithParameters?token=b204eee759ab38ebb986d223f6c5b4ce&commitid=$line"


please use powershell cmd-lets, not aliases that are "linux-like"

This is a linux script and is run on jenkins slave

ociuhandu · 2017-02-01T15:10:36Z

scripts/unit-tests/linux/ovs-git.sh

+    if [ ! -e $lastranfile ]; then
+        echo "LastRanFile doesn't exist. Creating it."
+        cd $WORKSPACE/ovs
+        git rev-parse HEAD | tee $WORKSPACE/lastrancommit.txt


shouldn't you run a test on that "first commit"?

Done. Now it will run the "first commit" test

ociuhandu · 2017-02-01T16:57:21Z

scripts/unit-tests/windows/ovs-email.ps1

+
+if ( $Status -eq "ERROR" ) {
+        write-host "Sending make error E-mail for $commitID"
+        Send-PassEmail $from "mgheorghe@cloudbasesolutions.com" $smtpServer $commitLink $logLink


Shouldn't "$to" be listed here as well?

ociuhandu · 2017-02-01T16:58:31Z

scripts/unit-tests/windows/ovs-req.ps1

@@ -0,0 +1,110 @@
+$ErrorActionPreference = "Stop"


Is this script required / executed for each run?

nope, removed

ociuhandu · 2017-02-01T17:22:50Z

jjb/ovs-unit.yaml

+      - shell: |
+          #!/bin/bash
+          set -e
+          function Check-LastRanFile {


why are these functions defined both here and in scripts/unit-tests/linux/ovs-git.sh?

this will not be present in the jjb

ociuhandu · 2017-02-01T17:26:52Z

scripts/unit-tests/windows/ovs-email.ps1

+
+$Status = $env:status
+$commitID = $env:commitid
+$basePath = "C:\OpenStack\OpenvSwitch"


Again, use config.ps1, no static variables defined in main file.

ociuhandu · 2017-02-01T17:28:13Z

scripts/unit-tests/windows/ovs-email.ps1

@@ -0,0 +1,87 @@
+function Send-PassEmail ($from, $to, $smtpServer, $commitLink, $logLink) {


Why not a function that takes as parameter also the PASS/FAIL/ERROR status? What's the significant difference between them?

Combined them into one functions "Send-Email"

…d jjb file since it will be hosted in another repo

OVS unit tests. JJB yaml file.

bbf1b09

ociuhandu requested changes Feb 6, 2017

View reviewed changes

ociuhandu assigned capsali Feb 10, 2017

Changed directory structure. Made changes according to review. Remove…

1e8dd65

…d jjb file since it will be hosted in another repo

		@@ -0,0 +1,87 @@
		function Send-PassEmail ($from, $to, $smtpServer, $commitLink, $logLink) {

OVS unit tests. JJB yaml file. #1

Are you sure you want to change the base?

OVS unit tests. JJB yaml file. #1

Uh oh!

Conversation

capsali commented Feb 1, 2017

Uh oh!

ociuhandu commented Feb 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ociuhandu commented Feb 1, 2017 •

edited

Loading