[ 
https://issues.apache.org/jira/browse/FLINK-24270?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Dawid Wysakowicz updated FLINK-24270:
-------------------------------------
    Description: 
>From https://github.com/apache/flink/pull/16655#issuecomment-899603149:

{quote}
All the tests about checking the illegal JobGraph modifications are written as 
tests against the CheckpointCoordinator, when they could be written just 
against the VertexFinishedStateChecker. That would make the tests more 
targeted, like only against the actual component that has the logic. That way, 
we need less test maintenance when the checkpoint coordinator changes later.

It is probably a good idea to have two test against the Scheduler that validate 
that the modification tests happen at the right points. Something like 
testJobGraphModificationsAreCheckedForInitialSavepoint() and 
testJobGraphModificationsAreCheckedForInitialCheckpoint().

Then we need no dedicated tests against the CheckpointCoordinator regarding 
illegal job upgrades. That makes sense, because handling this is also the 
responsibilities of the Scheduler and the VertexFinishedStateChecker. The 
CheckpointCoordinator is only the component that connects the two, and forwards 
the calls.
{quote}

  was:
>From https://github.com/apache/flink/pull/16655#issuecomment-899603149:

All the tests about checking the illegal JobGraph modifications are written as 
tests against the CheckpointCoordinator, when they could be written just 
against the VertexFinishedStateChecker. That would make the tests more 
targeted, like only against the actual component that has the logic. That way, 
we need less test maintenance when the checkpoint coordinator changes later.

It is probably a good idea to have two test against the Scheduler that validate 
that the modification tests happen at the right points. Something like 
testJobGraphModificationsAreCheckedForInitialSavepoint() and 
testJobGraphModificationsAreCheckedForInitialCheckpoint().

Then we need no dedicated tests against the CheckpointCoordinator regarding 
illegal job upgrades. That makes sense, because handling this is also the 
responsibilities of the Scheduler and the VertexFinishedStateChecker. The 
CheckpointCoordinator is only the component that connects the two, and forwards 
the calls.


> Rewrite tests for illegal job modification against VertexFinishedStateChecker
> -----------------------------------------------------------------------------
>
>                 Key: FLINK-24270
>                 URL: https://issues.apache.org/jira/browse/FLINK-24270
>             Project: Flink
>          Issue Type: Sub-task
>          Components: Runtime / Checkpointing
>            Reporter: Dawid Wysakowicz
>            Priority: Major
>             Fix For: 1.15.0
>
>
> From https://github.com/apache/flink/pull/16655#issuecomment-899603149:
> {quote}
> All the tests about checking the illegal JobGraph modifications are written 
> as tests against the CheckpointCoordinator, when they could be written just 
> against the VertexFinishedStateChecker. That would make the tests more 
> targeted, like only against the actual component that has the logic. That 
> way, we need less test maintenance when the checkpoint coordinator changes 
> later.
> It is probably a good idea to have two test against the Scheduler that 
> validate that the modification tests happen at the right points. Something 
> like testJobGraphModificationsAreCheckedForInitialSavepoint() and 
> testJobGraphModificationsAreCheckedForInitialCheckpoint().
> Then we need no dedicated tests against the CheckpointCoordinator regarding 
> illegal job upgrades. That makes sense, because handling this is also the 
> responsibilities of the Scheduler and the VertexFinishedStateChecker. The 
> CheckpointCoordinator is only the component that connects the two, and 
> forwards the calls.
> {quote}



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to