Skip to content

Commit 0055756

Browse files
forsaken628andreyvelich
forsaken628
authored and
andreyvelich
committed
Fix TestReconcileBatchJob (kubeflow#2350)
* update Signed-off-by: forsaken628 <[email protected]> * fix Signed-off-by: forsaken628 <[email protected]> * update Signed-off-by: forsaken628 <[email protected]> * update Signed-off-by: forsaken628 <[email protected]> * update Signed-off-by: forsaken628 <[email protected]> * fix Signed-off-by: forsaken628 <[email protected]> * cleanup Signed-off-by: forsaken628 <[email protected]> * fix Signed-off-by: forsaken628 <[email protected]> * update Signed-off-by: forsaken628 <[email protected]> * use gomock Signed-off-by: forsaken628 <[email protected]> --------- Signed-off-by: forsaken628 <[email protected]>
1 parent 6cac704 commit 0055756

File tree

1 file changed

+137
-121
lines changed

1 file changed

+137
-121
lines changed

‎pkg/controller.v1beta1/trial/trial_controller_test.go‎

Lines changed: 137 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ var trialKey = types.NamespacedName{Name: trialName, Namespace: namespace}
5858
var batchJobKey = types.NamespacedName{Name: batchJobName, Namespace: namespace}
5959

6060
func init() {
61-
logf.SetLogger(zap.New())
61+
logf.SetLogger(zap.New(zap.UseDevMode(true)))
6262
}
6363

6464
func TestAdd(t *testing.T) {
@@ -179,137 +179,153 @@ func TestReconcileBatchJob(t *testing.T) {
179179
},
180180
}
181181

182-
mockManagerClient.EXPECT().GetTrialObservationLog(gomock.Any()).Return(observationLogAvailable, nil).Times(1)
183-
mockManagerClient.EXPECT().GetTrialObservationLog(gomock.Any()).Return(observationLogUnavailable, nil).MinTimes(1)
184-
mockManagerClient.EXPECT().DeleteTrialObservationLog(gomock.Any()).Return(nil, nil).AnyTimes()
182+
t.Run(`Trial run with "Failed" BatchJob.`, func(t *testing.T) {
183+
g := gomega.NewGomegaWithT(t)
184+
mockManagerClient.EXPECT().DeleteTrialObservationLog(gomock.Any()).Return(nil, nil)
185185

186-
// Test 1 - Trial run with "Failed" BatchJob.
187-
trial := newFakeTrialBatchJob()
188-
batchJob := &batchv1.Job{}
186+
trial := newFakeTrialBatchJob()
187+
batchJob := &batchv1.Job{}
189188

190-
// Create the Trial
191-
g.Expect(c.Create(ctx, trial)).NotTo(gomega.HaveOccurred())
189+
// Create the Trial
190+
g.Expect(c.Create(ctx, trial)).NotTo(gomega.HaveOccurred())
192191

193-
// Expect that BatchJob with appropriate name is created
194-
g.Eventually(func() error {
195-
return c.Get(ctx, batchJobKey, batchJob)
196-
}, timeout).Should(gomega.Succeed())
192+
// Expect that BatchJob with appropriate name is created
193+
g.Eventually(func() error {
194+
return c.Get(ctx, batchJobKey, batchJob)
195+
}, timeout).Should(gomega.Succeed())
197196

198-
// Expect that Trial status is running
199-
g.Eventually(func() bool {
200-
if err = c.Get(ctx, trialKey, trial); err != nil {
201-
return false
202-
}
203-
return trial.IsRunning()
204-
}, timeout).Should(gomega.BeTrue())
205-
206-
// Manually update BatchJob status to failed
207-
// Expect that Trial status is failed
208-
g.Eventually(func() bool {
209-
if err = c.Get(ctx, batchJobKey, batchJob); err != nil {
210-
return false
211-
}
197+
// Expect that Trial status is running
198+
g.Eventually(func() bool {
199+
if err = c.Get(ctx, trialKey, trial); err != nil {
200+
return false
201+
}
202+
return trial.IsRunning()
203+
}, timeout).Should(gomega.BeTrue())
204+
205+
// Manually update BatchJob status to failed
206+
// Expect that Trial status is failed
207+
g.Eventually(func() bool {
208+
if err = c.Get(ctx, batchJobKey, batchJob); err != nil {
209+
return false
210+
}
211+
batchJob.Status = batchv1.JobStatus{
212+
Conditions: []batchv1.JobCondition{
213+
{
214+
Type: batchv1.JobFailed,
215+
Status: corev1.ConditionTrue,
216+
Message: "BatchJob failed test message",
217+
Reason: "BatchJob failed test reason",
218+
},
219+
},
220+
}
221+
if err = c.Status().Update(ctx, batchJob); err != nil {
222+
return false
223+
}
224+
225+
if err = c.Get(ctx, trialKey, trial); err != nil {
226+
return false
227+
}
228+
return trial.IsFailed()
229+
}, timeout).Should(gomega.BeTrue())
230+
231+
// Delete the Trial
232+
g.Expect(c.Delete(ctx, trial)).NotTo(gomega.HaveOccurred())
233+
234+
// Expect that Trial is deleted
235+
// BatchJob can't be deleted because GC doesn't work in envtest and BatchJob stuck in termination phase.
236+
// Ref: https://book.kubebuilder.io/reference/testing/envtest.html#testing-considerations.
237+
g.Eventually(func() bool {
238+
return errors.IsNotFound(c.Get(ctx, trialKey, &trialsv1beta1.Trial{}))
239+
}, timeout).Should(gomega.BeTrue())
240+
})
241+
242+
t.Run(`Trail with "Complete" BatchJob and Available metrics.`, func(t *testing.T) {
243+
g := gomega.NewGomegaWithT(t)
244+
gomock.InOrder(
245+
mockManagerClient.EXPECT().GetTrialObservationLog(gomock.Any()).Return(observationLogAvailable, nil).MinTimes(1),
246+
mockManagerClient.EXPECT().DeleteTrialObservationLog(gomock.Any()).Return(nil, nil),
247+
)
248+
batchJob := &batchv1.Job{}
249+
batchJobCompleteMessage := "BatchJob completed test message"
250+
batchJobCompleteReason := "BatchJob completed test reason"
251+
// Update BatchJob status to Complete.
252+
g.Expect(c.Get(ctx, batchJobKey, batchJob)).NotTo(gomega.HaveOccurred())
212253
batchJob.Status = batchv1.JobStatus{
213254
Conditions: []batchv1.JobCondition{
214255
{
215-
Type: batchv1.JobFailed,
256+
Type: batchv1.JobComplete,
216257
Status: corev1.ConditionTrue,
217-
Message: "BatchJob failed test message",
218-
Reason: "BatchJob failed test reason",
258+
Message: batchJobCompleteMessage,
259+
Reason: batchJobCompleteReason,
219260
},
220261
},
221262
}
222-
if err = c.Status().Update(ctx, batchJob); err != nil {
223-
return false
224-
}
225-
226-
if err = c.Get(ctx, trialKey, trial); err != nil {
227-
return false
228-
}
229-
return trial.IsFailed()
230-
}, timeout).Should(gomega.BeTrue())
231-
232-
// Delete the Trial
233-
g.Expect(c.Delete(ctx, trial)).NotTo(gomega.HaveOccurred())
234-
235-
// Expect that Trial is deleted
236-
// BatchJob can't be deleted because GC doesn't work in envtest and BatchJob stuck in termination phase.
237-
// Ref: https://book.kubebuilder.io/reference/testing/envtest.html#testing-considerations.
238-
g.Eventually(func() bool {
239-
return errors.IsNotFound(c.Get(ctx, trialKey, &trialsv1beta1.Trial{}))
240-
}, timeout).Should(gomega.BeTrue())
241-
242-
// Test 2 - Trail with "Complete" BatchJob and Available metrics.
243-
batchJobCompleteMessage := "BatchJob completed test message"
244-
batchJobCompleteReason := "BatchJob completed test reason"
245-
// Update BatchJob status to Complete.
246-
g.Expect(c.Get(ctx, batchJobKey, batchJob)).NotTo(gomega.HaveOccurred())
247-
batchJob.Status = batchv1.JobStatus{
248-
Conditions: []batchv1.JobCondition{
249-
{
250-
Type: batchv1.JobComplete,
251-
Status: corev1.ConditionTrue,
252-
Message: batchJobCompleteMessage,
253-
Reason: batchJobCompleteReason,
254-
},
255-
},
256-
}
257-
g.Expect(c.Status().Update(ctx, batchJob)).NotTo(gomega.HaveOccurred())
258-
259-
// Create the Trial
260-
trial = newFakeTrialBatchJob()
261-
g.Expect(c.Create(ctx, trial)).NotTo(gomega.HaveOccurred())
262-
263-
// Expect that Trial status is succeeded and metrics are properly populated
264-
// Metrics available because GetTrialObservationLog returns values
265-
g.Eventually(func() bool {
266-
if err = c.Get(ctx, trialKey, trial); err != nil {
267-
return false
268-
}
269-
return trial.IsSucceeded() &&
270-
len(trial.Status.Observation.Metrics) > 0 &&
271-
trial.Status.Observation.Metrics[0].Min == "0.11" &&
272-
trial.Status.Observation.Metrics[0].Max == "0.99" &&
273-
trial.Status.Observation.Metrics[0].Latest == "0.11"
274-
}, timeout).Should(gomega.BeTrue())
275-
276-
// Delete the Trial
277-
g.Expect(c.Delete(ctx, trial)).NotTo(gomega.HaveOccurred())
278-
279-
// Expect that Trial is deleted
280-
g.Eventually(func() bool {
281-
return errors.IsNotFound(c.Get(ctx, trialKey, &trialsv1beta1.Trial{}))
282-
}, timeout).Should(gomega.BeTrue())
283-
284-
// Test 3 - Trail with "Complete" BatchJob and Unavailable metrics.
285-
// Create the Trial
286-
trial = newFakeTrialBatchJob()
287-
g.Expect(c.Create(ctx, trial)).NotTo(gomega.HaveOccurred())
288-
289-
// Expect that Trial status is succeeded with "false" status and "metrics unavailable" reason.
290-
// Metrics unavailable because GetTrialObservationLog returns "unavailable".
291-
g.Eventually(func() bool {
292-
if err = c.Get(ctx, trialKey, trial); err != nil {
293-
return false
294-
}
295-
return trial.IsMetricsUnavailable() &&
296-
len(trial.Status.Observation.Metrics) > 0 &&
297-
trial.Status.Observation.Metrics[0].Min == consts.UnavailableMetricValue &&
298-
trial.Status.Observation.Metrics[0].Max == consts.UnavailableMetricValue &&
299-
trial.Status.Observation.Metrics[0].Latest == consts.UnavailableMetricValue
300-
}, timeout).Should(gomega.BeTrue())
301-
302-
// Delete the Trial
303-
g.Expect(c.Delete(ctx, trial)).NotTo(gomega.HaveOccurred())
304-
305-
// Expect that Trial is deleted
306-
g.Eventually(func() bool {
307-
return errors.IsNotFound(c.Get(ctx, trialKey, &trialsv1beta1.Trial{}))
308-
}, timeout).Should(gomega.BeTrue())
309-
310-
// Test 4 - Update status for empty Trial
311-
g.Expect(r.updateStatus(&trialsv1beta1.Trial{})).To(gomega.HaveOccurred())
312-
263+
g.Expect(c.Status().Update(ctx, batchJob)).NotTo(gomega.HaveOccurred())
264+
265+
// Create the Trial
266+
trial := newFakeTrialBatchJob()
267+
g.Expect(c.Create(ctx, trial)).NotTo(gomega.HaveOccurred())
268+
269+
// Expect that Trial status is succeeded and metrics are properly populated
270+
// Metrics available because GetTrialObservationLog returns values
271+
start := time.Now()
272+
g.Eventually(func() bool {
273+
if err = c.Get(ctx, trialKey, trial); err != nil {
274+
t.Log(time.Since(start), err)
275+
return false
276+
}
277+
return trial.IsSucceeded() &&
278+
len(trial.Status.Observation.Metrics) > 0 &&
279+
trial.Status.Observation.Metrics[0].Min == "0.11" &&
280+
trial.Status.Observation.Metrics[0].Max == "0.99" &&
281+
trial.Status.Observation.Metrics[0].Latest == "0.11"
282+
}, timeout).Should(gomega.BeTrue())
283+
284+
// Delete the Trial
285+
g.Expect(c.Delete(ctx, trial)).NotTo(gomega.HaveOccurred())
286+
287+
// Expect that Trial is deleted
288+
g.Eventually(func() bool {
289+
return errors.IsNotFound(c.Get(ctx, trialKey, &trialsv1beta1.Trial{}))
290+
}, timeout).Should(gomega.BeTrue())
291+
})
292+
293+
t.Run(`Trail with "Complete" BatchJob and Unavailable metrics.`, func(t *testing.T) {
294+
g := gomega.NewGomegaWithT(t)
295+
gomock.InOrder(
296+
mockManagerClient.EXPECT().GetTrialObservationLog(gomock.Any()).Return(observationLogUnavailable, nil).MinTimes(1),
297+
mockManagerClient.EXPECT().DeleteTrialObservationLog(gomock.Any()).Return(nil, nil),
298+
)
299+
// Create the Trial
300+
trial := newFakeTrialBatchJob()
301+
g.Expect(c.Create(ctx, trial)).NotTo(gomega.HaveOccurred())
302+
303+
// Expect that Trial status is succeeded with "false" status and "metrics unavailable" reason.
304+
// Metrics unavailable because GetTrialObservationLog returns "unavailable".
305+
g.Eventually(func() bool {
306+
if err = c.Get(ctx, trialKey, trial); err != nil {
307+
return false
308+
}
309+
return trial.IsMetricsUnavailable() &&
310+
len(trial.Status.Observation.Metrics) > 0 &&
311+
trial.Status.Observation.Metrics[0].Min == consts.UnavailableMetricValue &&
312+
trial.Status.Observation.Metrics[0].Max == consts.UnavailableMetricValue &&
313+
trial.Status.Observation.Metrics[0].Latest == consts.UnavailableMetricValue
314+
}, timeout).Should(gomega.BeTrue())
315+
316+
// Delete the Trial
317+
g.Expect(c.Delete(ctx, trial)).NotTo(gomega.HaveOccurred())
318+
319+
// Expect that Trial is deleted
320+
g.Eventually(func() bool {
321+
return errors.IsNotFound(c.Get(ctx, trialKey, &trialsv1beta1.Trial{}))
322+
}, timeout).Should(gomega.BeTrue())
323+
})
324+
325+
t.Run("Update status for empty Trial", func(t *testing.T) {
326+
g := gomega.NewGomegaWithT(t)
327+
g.Expect(r.updateStatus(&trialsv1beta1.Trial{})).To(gomega.HaveOccurred())
328+
})
313329
}
314330

315331
func TestGetObjectiveMetricValue(t *testing.T) {

0 commit comments

Comments
 (0)