Skip to content
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

Fix unit tests and enable tests in CI for ai-video #3190

Merged
merged 27 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
729efd9
Do not set default max price to 0
leszko Oct 1, 2024
e91468f
Get back theMinLSSelector.Select() logic to what we have for transcoding
leszko Oct 1, 2024
34863f7
Fix unit tests
leszko Oct 2, 2024
a10d52b
Comment out failing tests
leszko Oct 2, 2024
835e681
Enable tests in CI
leszko Oct 2, 2024
c3e5b39
Merge branch 'ai-video' into rafal/ai-video-fix-unit-tests
leszko Oct 2, 2024
5907466
Revert changes in selection.go
leszko Oct 2, 2024
9e967b9
Skip failing tests
leszko Oct 2, 2024
2808bb0
Skip failing tests
leszko Oct 2, 2024
82b6661
Skip failing tests
leszko Oct 2, 2024
f75b64f
Update testcontainers dependency
leszko Oct 2, 2024
d7cc59c
Merge branch 'ai-video' into rafal/ai-video-fix-unit-tests
leszko Oct 2, 2024
c3721dd
Fix goleak error in tests
leszko Oct 2, 2024
e972e8c
Fix goleak error in tests
leszko Oct 2, 2024
89d2961
Merge branch 'ai-video' into rafal/ai-video-fix-unit-tests
leszko Oct 2, 2024
d526b18
Remove lint
leszko Oct 2, 2024
0f83bbd
Merge remote-tracking branch 'origin/rafal/ai-video-fix-unit-tests' i…
leszko Oct 2, 2024
26ab5d2
Removed test for deprecated segData.Profiles field + fix mediaserver_…
leszko Oct 3, 2024
8a08f88
Re-enable lint
leszko Oct 3, 2024
12f86e4
Change golint to revive
leszko Oct 3, 2024
c9d133a
Update test.yaml
leszko Oct 3, 2024
7ad48f7
golangci-lint
leszko Oct 3, 2024
cb4f93b
Fix lint errors
leszko Oct 3, 2024
3a5fdef
Merge remote-tracking branch 'upstream/ai-video' into rafal/ai-video-…
leszko Oct 21, 2024
ebe838b
Fix tests and lint
leszko Oct 21, 2024
e1469f7
Fix tests and lint
leszko Oct 21, 2024
60213e4
Fix OrchSecret == "" condition in starter
leszko Oct 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ on:
pull_request:
push:
branches:
# - master
- ai-video
tags:
- "v*"
Expand Down Expand Up @@ -338,7 +337,7 @@ jobs:
destination: "build.livepeer.live/${{ github.event.repository.name }}/ai-video/stable"
parent: false
process_gcloudignore: false

# Update the latest branch manifest
- name: Upload branch manifest file to Google Cloud stable folder
id: upload-manifest-latest
Expand Down
11 changes: 6 additions & 5 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
name: Trigger test suite

on:
# pull_request:
# branches:
# - master
pull_request:
branches:
- master
- ai-video
push:
branches:
- master
Expand Down Expand Up @@ -94,9 +95,9 @@ jobs:
- name: Lint
uses: golangci/golangci-lint-action@v4
with:
version: v1.52.2
version: v1.61.0
skip-pkg-cache: true
args: '--disable-all --enable=gofmt --enable=vet --enable=golint --deadline=4m pm verification'
args: '--out-format=colored-line-number --disable-all --enable=gofmt --enable=govet --enable=revive pm verification'
rickstaa marked this conversation as resolved.
Show resolved Hide resolved

- name: Run Revive Action by building from repository
uses: docker://morphy/revive-action:v2
Expand Down
2 changes: 1 addition & 1 deletion common/testutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
"github.com/livepeer/go-livepeer/server.(*LivepeerServer).StartMediaServer", "github.com/livepeer/go-livepeer/core.(*RemoteTranscoderManager).Manage.func1",
"github.com/livepeer/go-livepeer/server.(*LivepeerServer).HandlePush.func1", "github.com/rjeczalik/notify.(*nonrecursiveTree).dispatch",
"github.com/rjeczalik/notify.(*nonrecursiveTree).internal", "github.com/livepeer/lpms/stream.NewBasicRTMPVideoStream.func1", "github.com/patrickmn/go-cache.(*janitor).Run",
"github.com/golang/glog.(*fileSink).flushDaemon",
"github.com/golang/glog.(*fileSink).flushDaemon", "github.com/ipfs/go-log/writer.(*MirrorWriter).logRoutine",

Check warning on line 92 in common/testutil.go

View check run for this annotation

Codecov / codecov/patch

common/testutil.go#L92

Added line #L92 was not covered by tests
}

res := make([]goleak.Option, 0, len(funcs2ignore))
Expand Down
90 changes: 0 additions & 90 deletions common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
"sort"
"strconv"
"strings"
"testing"
"time"

"github.com/ethereum/go-ethereum/crypto"
"github.com/golang/glog"
"github.com/jaypipes/ghw"
"github.com/jaypipes/ghw/pkg/gpu"
"github.com/jaypipes/ghw/pkg/pci"
Expand Down Expand Up @@ -66,7 +64,6 @@

var (
ErrParseBigInt = fmt.Errorf("failed to parse big integer")
ErrProfile = fmt.Errorf("failed to parse profile")

ErrChromaFormat = fmt.Errorf("unknown VideoProfile ChromaFormat")
ErrFormatProto = fmt.Errorf("unknown VideoProfile format for protobufs")
Expand Down Expand Up @@ -99,93 +96,6 @@
}
}

func WaitUntil(waitTime time.Duration, condition func() bool) {
start := time.Now()
for time.Since(start) < waitTime {
if condition() == false {
time.Sleep(100 * time.Millisecond)
continue
}
break
}
}

func WaitAssert(t *testing.T, waitTime time.Duration, condition func() bool, msg string) {
start := time.Now()
for time.Since(start) < waitTime {
if condition() == false {
time.Sleep(100 * time.Millisecond)
continue
}
break
}

if condition() == false {
t.Errorf(msg)
}
}

func Retry(attempts int, sleep time.Duration, fn func() error) error {
if err := fn(); err != nil {
if attempts--; attempts > 0 {
time.Sleep(sleep)
return Retry(attempts, 2*sleep, fn)
}
return err
}

return nil
}

func TxDataToVideoProfile(txData string) ([]ffmpeg.VideoProfile, error) {
profiles := make([]ffmpeg.VideoProfile, 0)

if len(txData) == 0 {
return profiles, nil
}
if len(txData) < VideoProfileIDSize {
return nil, ErrProfile
}

for i := 0; i+VideoProfileIDSize <= len(txData); i += VideoProfileIDSize {
txp := txData[i : i+VideoProfileIDSize]

p, ok := ffmpeg.VideoProfileLookup[VideoProfileNameLookup[txp]]
if !ok {
glog.Errorf("Cannot find video profile for job: %v", txp)
return nil, ErrProfile // monitor to see if this is too aggressive
}
profiles = append(profiles, p)
}

return profiles, nil
}

func BytesToVideoProfile(txData []byte) ([]ffmpeg.VideoProfile, error) {
profiles := make([]ffmpeg.VideoProfile, 0)

if len(txData) == 0 {
return profiles, nil
}
if len(txData) < VideoProfileIDBytes {
return nil, ErrProfile
}

for i := 0; i+VideoProfileIDBytes <= len(txData); i += VideoProfileIDBytes {
var txp [VideoProfileIDBytes]byte
copy(txp[:], txData[i:i+VideoProfileIDBytes])

p, ok := ffmpeg.VideoProfileLookup[VideoProfileByteLookup[txp]]
if !ok {
glog.Errorf("Cannot find video profile for job: %v", txp)
return nil, ErrProfile // monitor to see if this is too aggressive
}
profiles = append(profiles, p)
}

return profiles, nil
}

func FFmpegProfiletoNetProfile(ffmpegProfiles []ffmpeg.VideoProfile) ([]*net.VideoProfile, error) {
profiles := make([]*net.VideoProfile, 0, len(ffmpegProfiles))
for _, profile := range ffmpegProfiles {
Expand Down Expand Up @@ -493,7 +403,7 @@
if len(cards) != 0 {
for _, card := range cards {
if card.DeviceInfo != nil && re.MatchString(card.DeviceInfo.Vendor.Name) {
nvidiaCardCount += 1

Check warning on line 406 in common/util.go

View workflow job for this annotation

GitHub Actions / Run tests defined for the project

should replace nvidiaCardCount += 1 with nvidiaCardCount++
}
}
} else { // on VMs gpu.GraphicsCards may be empty
Expand All @@ -509,7 +419,7 @@
// On some VMs driver may be misreported as vfio-pci, try to rely on device.Class.Name with a "Display controller"
// See: https://github.com/jaypipes/ghw/issues/314#issuecomment-1113334378
if device.Vendor != nil && re.MatchString(device.Vendor.Name) && (re.MatchString(device.Driver) || rePCI.MatchString(device.Class.Name)) {
nvidiaCardCount += 1

Check warning on line 422 in common/util.go

View workflow job for this annotation

GitHub Actions / Run tests defined for the project

should replace nvidiaCardCount += 1 with nvidiaCardCount++
}
}
}
Expand Down
60 changes: 0 additions & 60 deletions common/util_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package common

import (
"encoding/hex"
"fmt"
"math"
"math/big"
Expand All @@ -19,45 +18,6 @@ import (
"github.com/stretchr/testify/assert"
)

func TestTxDataToVideoProfile(t *testing.T) {
if res, err := TxDataToVideoProfile(""); err != nil && len(res) != 0 {
t.Error("Unexpected return on empty input")
}
if _, err := TxDataToVideoProfile("abc"); err != ErrProfile {
t.Error("Unexpected return on too-short input", err)
}
if _, err := TxDataToVideoProfile("abcdefghijk"); err != ErrProfile {
t.Error("Unexpected return on invalid input", err)
}
res, err := TxDataToVideoProfile("93c717e7c0a6517a")
if err != nil || res[1] != ffmpeg.P240p30fps16x9 || res[0] != ffmpeg.P360p30fps16x9 {
t.Error("Unexpected profile! ", err, res)
}
}

func TestVideoProfileBytes(t *testing.T) {
if len(VideoProfileByteLookup) != len(VideoProfileNameLookup) {
t.Error("Video profile byte map was not created correctly")
}
if res, err := BytesToVideoProfile(nil); err != nil && len(res) != 0 {
t.Error("Unexpected return on empty input")
}
if res, err := BytesToVideoProfile([]byte{}); err != nil && len(res) != 0 {
t.Error("Unexpected return on empty input")
}
if _, err := BytesToVideoProfile([]byte("abc")); err != ErrProfile {
t.Error("Unexpected return on too-short input", err)
}
if _, err := BytesToVideoProfile([]byte("abcdefghijk")); err != ErrProfile {
t.Error("Unexpected return on invalid input", err)
}
b, _ := hex.DecodeString("93c717e7c0a6517a")
res, err := BytesToVideoProfile(b)
if err != nil || res[1] != ffmpeg.P240p30fps16x9 || res[0] != ffmpeg.P360p30fps16x9 {
t.Error("Unexpected profile! ", err, res)
}
}

func TestFFmpegProfiletoNetProfile(t *testing.T) {
assert := assert.New(t)

Expand Down Expand Up @@ -158,26 +118,6 @@ func TestFFmpegProfiletoNetProfile(t *testing.T) {
assert.Nil(fullProfiles)
}

func TestProfilesToHex(t *testing.T) {
assert := assert.New(t)
// Sanity checking against an existing eth impl that we know works
compare := func(profiles []ffmpeg.VideoProfile) {
pCopy := make([]ffmpeg.VideoProfile, len(profiles))
copy(pCopy, profiles)
b1, err := hex.DecodeString(ProfilesToHex(profiles))
assert.Nil(err, "Error hex encoding/decoding")
b2, err := BytesToVideoProfile(b1)
assert.Nil(err, "Error converting back to profile")
assert.Equal(pCopy, b2)
}
// XXX double check which one is wrong! ethcommon method produces "0" zero string
// compare(nil)
// compare([]ffmpeg.VideoProfile{})
compare([]ffmpeg.VideoProfile{ffmpeg.P240p30fps16x9})
compare([]ffmpeg.VideoProfile{ffmpeg.P240p30fps16x9, ffmpeg.P360p30fps16x9})
compare([]ffmpeg.VideoProfile{ffmpeg.P360p30fps16x9, ffmpeg.P240p30fps16x9})
}

func TestVideoProfile_FormatMimeType(t *testing.T) {
inp := []ffmpeg.Format{ffmpeg.FormatNone, ffmpeg.FormatMPEGTS, ffmpeg.FormatMP4}
exp := []string{"video/mp2t", "video/mp2t", "video/mp4"}
Expand Down
34 changes: 19 additions & 15 deletions core/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,16 +489,18 @@
for capability, capacity := range c.capacities {
netCaps.Capacities[uint32(capability)] = uint32(capacity)
}
for capability, constraints := range c.constraints.perCapability {
models := make(map[string]*net.Capabilities_CapabilityConstraints_ModelConstraint)
for modelID, modelConstraint := range constraints.Models {
models[modelID] = &net.Capabilities_CapabilityConstraints_ModelConstraint{
Warm: modelConstraint.Warm,
if c.constraints.perCapability != nil {
for capability, constraints := range c.constraints.perCapability {
models := make(map[string]*net.Capabilities_CapabilityConstraints_ModelConstraint)
for modelID, modelConstraint := range constraints.Models {
models[modelID] = &net.Capabilities_CapabilityConstraints_ModelConstraint{
Warm: modelConstraint.Warm,

Check warning on line 497 in core/capabilities.go

View check run for this annotation

Codecov / codecov/patch

core/capabilities.go#L494-L497

Added lines #L494 - L497 were not covered by tests
}
}
}

netCaps.Constraints.PerCapability[uint32(capability)] = &net.Capabilities_CapabilityConstraints{
Models: models,
netCaps.Constraints.PerCapability[uint32(capability)] = &net.Capabilities_CapabilityConstraints{
Models: models,

Check warning on line 502 in core/capabilities.go

View check run for this annotation

Codecov / codecov/patch

core/capabilities.go#L501-L502

Added lines #L501 - L502 were not covered by tests
}
}
}
return netCaps
Expand Down Expand Up @@ -531,14 +533,16 @@
}
}

for capabilityInt, constraints := range caps.Constraints.PerCapability {
models := make(map[string]*ModelConstraint)
for modelID, modelConstraint := range constraints.Models {
models[modelID] = &ModelConstraint{Warm: modelConstraint.Warm}
}
if caps.Constraints != nil && caps.Constraints.PerCapability != nil {
for capabilityInt, constraints := range caps.Constraints.PerCapability {
models := make(map[string]*ModelConstraint)
for modelID, modelConstraint := range constraints.Models {
models[modelID] = &ModelConstraint{Warm: modelConstraint.Warm}

Check warning on line 540 in core/capabilities.go

View check run for this annotation

Codecov / codecov/patch

core/capabilities.go#L538-L540

Added lines #L538 - L540 were not covered by tests
}

coreCaps.constraints.perCapability[Capability(capabilityInt)] = &CapabilityConstraints{
Models: models,
coreCaps.constraints.perCapability[Capability(capabilityInt)] = &CapabilityConstraints{
Models: models,

Check warning on line 544 in core/capabilities.go

View check run for this annotation

Codecov / codecov/patch

core/capabilities.go#L543-L544

Added lines #L543 - L544 were not covered by tests
}
}
}

Expand Down
Loading
Loading