From a56ce1dba300b711af727edb5053ef19f8127bf6 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 17 Oct 2016 13:50:06 -0700 Subject: [PATCH 1/3] Added error message when build fails and there is no dockerfile in a specific branch --- .../buildLogs/buildLogsController.js | 13 ++++++++++- .../buildLogs/buildLogsController.unit.js | 22 +++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/client/directives/components/buildLogs/buildLogsController.js b/client/directives/components/buildLogs/buildLogsController.js index 34090d17e..363e12433 100644 --- a/client/directives/components/buildLogs/buildLogsController.js +++ b/client/directives/components/buildLogs/buildLogsController.js @@ -7,11 +7,13 @@ function BuildLogsController( $rootScope, $timeout, errs, + keypather, launchDebugContainer, loading, updateInstanceWithNewBuild, primus, promisify, + fetchRepoDockerfile, streamingLog ) { var BLC = this; @@ -27,10 +29,19 @@ function BuildLogsController( BLC.failReason = buildError.message || 'failed'; BLC.showDebug = true; BLC.buildLogsRunning = false; - BLC.showNoDockerfileError = (BLC.instance.hasDockerfileMirroring() && BLC.instance.mirroredDockerfile === null); if (status === 'neverStarted') { BLC.showErrorPanel = true; } + var repoAndBranchName = BLC.instance.getRepoAndBranchName().split('/'); + var repoName = repoAndBranchName[0]; + var branchName = repoAndBranchName[1]; + var dockerfilePath = keypather.get(BLC, 'instance.mirroredDockerfile.attrs.path') + keypather.get(BLC, 'instance.mirroredDockerfile.attrs.name'); + if (!!dockerfilePath) { + fetchRepoDockerfile(repoName, branchName, dockerfilePath) + .then(function (dockerfile) { + BLC.showNoDockerfileError = (BLC.instance.hasDockerfileMirroring() && dockerfile.message === 'Not Found'); + }) + } } else if (status === 'building') { BLC.buildStatus = 'running'; BLC.buildLogsRunning = true; diff --git a/test/unit/directives/components/buildLogs/buildLogsController.unit.js b/test/unit/directives/components/buildLogs/buildLogsController.unit.js index c3ebf70c0..2eec2668c 100644 --- a/test/unit/directives/components/buildLogs/buildLogsController.unit.js +++ b/test/unit/directives/components/buildLogs/buildLogsController.unit.js @@ -17,8 +17,10 @@ describe('BuildLogsController'.bold.underline.blue, function () { var BLC; var mockCreateDebugContainer; var mockDebugContainer; + var mockDockerfile; var mockErrs; var isRunnabotPartOfOrgStub; + var fetchRepoDockerfileStub; function setup(useInstance, dontIncludeBuildDockerContainer) { mockInstance = { @@ -36,6 +38,13 @@ describe('BuildLogsController'.bold.underline.blue, function () { dockerContainer: dontIncludeBuildDockerContainer ? undefined : 'asdsdfsd' } } + }, + getRepoAndBranchName: sinon.stub().returns('test/master'), + mirroredDockerfile: { + attrs: { + path: '/', + name: 'Dockerfile' + } } }; mockDebugContainer = { @@ -51,7 +60,7 @@ describe('BuildLogsController'.bold.underline.blue, function () { logs: [] }; mockStreamingLog = sinon.stub().returns(mockStreamingLogContents); - + mockDockerfile = {message: 'Not Found'}; mockPrimus = { createBuildStream: sinon.spy(function () { mockStream = new EventEmitter(); @@ -77,7 +86,10 @@ describe('BuildLogsController'.bold.underline.blue, function () { mockCreateDebugContainer = sinon.stub().returns($q.when(mockDebugContainer)); return mockCreateDebugContainer; }); - + $provide.factory('fetchRepoDockerfile', function ($q) { + fetchRepoDockerfileStub = sinon.stub().returns($q.when(mockDockerfile)); + return fetchRepoDockerfileStub; + }); $provide.factory('isRunnabotPartOfOrg', function ($q) { isRunnabotPartOfOrgStub = sinon.stub().returns($q.when()); return isRunnabotPartOfOrgStub; @@ -188,6 +200,12 @@ describe('BuildLogsController'.bold.underline.blue, function () { mockInstance.on.lastCall.args[1](); expect(BLC.showDebug).to.be.ok; }); + it('should display a dockerfile error if absent', function () { + mockInstance.status.returns('buildFailed'); + mockInstance.on.lastCall.args[1](); + $scope.$digest(); + expect(BLC.showNoDockerfileError).to.equal(true); + }) }); describe('actions', function () { describe('launchDebugContainer', function () { From dceb13d63bf227815bf44f2bac8555e5dfa5543b Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 17 Oct 2016 13:51:28 -0700 Subject: [PATCH 2/3] Fixed linting --- client/directives/components/buildLogs/buildLogsController.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/directives/components/buildLogs/buildLogsController.js b/client/directives/components/buildLogs/buildLogsController.js index 363e12433..c85543150 100644 --- a/client/directives/components/buildLogs/buildLogsController.js +++ b/client/directives/components/buildLogs/buildLogsController.js @@ -40,7 +40,7 @@ function BuildLogsController( fetchRepoDockerfile(repoName, branchName, dockerfilePath) .then(function (dockerfile) { BLC.showNoDockerfileError = (BLC.instance.hasDockerfileMirroring() && dockerfile.message === 'Not Found'); - }) + }); } } else if (status === 'building') { BLC.buildStatus = 'running'; From a2a138015dfd3f2f8e912b7fb0c8afe75903f796 Mon Sep 17 00:00:00 2001 From: Henry Mollman Date: Mon, 16 Jan 2017 10:45:30 -0800 Subject: [PATCH 3/3] PR comments --- .../directives/components/buildLogs/buildLogsController.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/directives/components/buildLogs/buildLogsController.js b/client/directives/components/buildLogs/buildLogsController.js index c85543150..49ffdb765 100644 --- a/client/directives/components/buildLogs/buildLogsController.js +++ b/client/directives/components/buildLogs/buildLogsController.js @@ -35,9 +35,10 @@ function BuildLogsController( var repoAndBranchName = BLC.instance.getRepoAndBranchName().split('/'); var repoName = repoAndBranchName[0]; var branchName = repoAndBranchName[1]; - var dockerfilePath = keypather.get(BLC, 'instance.mirroredDockerfile.attrs.path') + keypather.get(BLC, 'instance.mirroredDockerfile.attrs.name'); - if (!!dockerfilePath) { - fetchRepoDockerfile(repoName, branchName, dockerfilePath) + var dockerfilePath = keypather.get(BLC, 'instance.mirroredDockerfile.attrs.path'); + var dockerfileName = keypather.get(BLC, 'instance.mirroredDockerfile.attrs.name'); + if (dockerfilePath && dockerfileName) { + fetchRepoDockerfile(repoName, branchName, dockerfilePath + dockerfileName) .then(function (dockerfile) { BLC.showNoDockerfileError = (BLC.instance.hasDockerfileMirroring() && dockerfile.message === 'Not Found'); });