-
Notifications
You must be signed in to change notification settings - Fork 15
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
ADM-709:[backend][docs]:Verify buildkite and obtain buildkite data with new API #889
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesYou may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation |
Hi @Andrea2000728! 👋 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor with the comments
@@ -35,6 +36,26 @@ public BuildKiteResponseDTO getBuildKiteInfo(@PathVariable String pipelineType, | |||
return buildKiteService.fetchPipelineInfo(pipelineParam); | |||
} | |||
|
|||
@PostMapping("/{pipelineType}/verify") | |||
public ResponseEntity<Void> verifyBuildKiteToken(@PathVariable String pipelineType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipelineType should be a enum
backend/src/main/java/heartbeat/service/pipeline/buildkite/BuildKiteService.java
Outdated
Show resolved
Hide resolved
} | ||
|
||
@PostMapping("/{pipelineType}/info") | ||
public ResponseEntity<BuildKiteResponseDTO> fetchBuildKiteInfo(@PathVariable String pipelineType, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pipelineType to a enum
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and remove /info
in the path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"/{xxxxType}/info" We have designed this API to all follow this form (pipeline board source). also the old api hasn't been removed yet, we can distinguish by this.
backend/src/main/java/heartbeat/service/pipeline/buildkite/BuildKiteService.java
Outdated
Show resolved
Hide resolved
backend/src/main/java/heartbeat/service/pipeline/buildkite/BuildKiteService.java
Outdated
Show resolved
Hide resolved
backend/src/test/java/heartbeat/controller/pipeline/BuildKiteControllerTest.java
Outdated
Show resolved
Hide resolved
void shouldReturnTrueWhenTokenIsCorrect() { | ||
BuildKiteTokenInfo buildKiteTokenInfo = BuildKiteTokenInfo.builder() | ||
.scopes(List.of("read_builds", "read_organizations", "read_pipelines")) | ||
.build(); | ||
|
||
when(buildKiteFeignClient.getTokenInfo(any())).thenReturn(buildKiteTokenInfo); | ||
buildKiteService.getBuildKiteVerify("mock_token"); | ||
|
||
verify(buildKiteFeignClient, times(1)).getTokenInfo(anyString()); | ||
} | ||
|
||
@Test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
given a new line between given when then
PipelineParam pipelineParam = PipelineParam.builder() | ||
.token("test_token") | ||
.startTime("startTime") | ||
.endTime("endTime") | ||
.build(); | ||
ObjectMapper mapper = new ObjectMapper(); | ||
List<BuildKitePipelineDTO> pipelineDTOS = mapper.readValue( | ||
new File("src/test/java/heartbeat/controller/pipeline/buildKitePipelineInfoData.json"), | ||
new TypeReference<>() { | ||
}); | ||
|
||
when(buildKiteFeignClient.getBuildKiteOrganizationsInfo(any())) | ||
.thenReturn(List.of(BuildKiteOrganizationsInfo.builder().name("XXXX").slug("XXXX").build())); | ||
when(buildKiteFeignClient.getPipelineInfo("Bearer test_token", "XXXX", "1", "100", "startTime", "endTime")) | ||
.thenReturn(pipelineDTOS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
those part should be in when
, combine it
@@ -12,6 +12,7 @@ | |||
import static org.mockito.ArgumentMatchers.any; | |||
import static org.mockito.ArgumentMatchers.anyString; | |||
import static org.mockito.Mockito.mock; | |||
import static org.mockito.Mockito.times; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactor this file with given when then
@@ -189,6 +190,7 @@ responses:{ | |||
] | |||
}] | |||
} | |||
当 pipelineList 为空时, responses 204 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
covert it into english
all done |
…th new API (#889) * ADM-709:[backend]feat: add verify token and get info for buildKite controller Co-authored-by: Andrea <Andrea2000728>
Summary
add two api:
1.Verify the BuildKite token
2.getBuildKiteInfo
Before
Description
Screenshots
If applicable, add screenshots to help explain behavior of your code.
After
Description
Screenshots
If applicable, add screenshots to help explain behavior of your code.
Note
Null