-
Notifications
You must be signed in to change notification settings - Fork 349
Development: Improve Programming Exercise export tests
#11392
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
base: develop
Are you sure you want to change the base?
Changes from all commits
c06832d
2e196a4
9d00023
73c8a7d
707637e
0356671
6a0d3f2
0767f61
f5f5a87
d2dc5f1
e218bd7
b5beb91
51c8cca
42dfaa4
0eecf38
1da53b5
3eb6dde
f12077f
6fa9b85
e437a1d
3c0d266
4f00aeb
28483ea
7945c00
c1b326f
c366f1e
7e33edc
1de24ff
d5ba771
4127676
ae9ee78
457508b
9bbfc4b
2e13784
ded36b3
99efbf8
52b19d2
f3f0c35
2911656
8c0ee2c
87ffab9
23b7eee
323374c
efa540c
e1c9444
dbc0913
df17fae
8d31d0b
742197c
ceb168d
6e4f64e
7cf6802
c0e91e5
83638d2
4fd765f
4e4f5d2
464345f
33e85e6
c02029f
6ba1957
9cb68e7
0d7ffe7
82c3228
5a78006
e176532
cfee00a
0d81396
a020143
a495d44
4efbb43
c89b3e0
e6db107
fa37d23
14486ea
ace53f5
95b7bd5
4ccc8bf
2d3f429
2e00db0
34f9707
e5d9630
685b843
b2d3473
431c075
c40ec2e
4ddeadd
ae24c88
5753e41
3505f6f
120e9a9
db58d3c
848138f
1cc5e86
40ce916
906ed8e
47e8641
970f7bc
1336f73
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1239,6 +1239,10 @@ public Map<File, FileType> listFilesAndFolders(Repository repo, boolean omitBina | |
| Iterator<java.io.File> itr = FileUtils.iterateFilesAndDirs(repo.getLocalPath().toFile(), filter, filter); | ||
| Map<File, FileType> files = new HashMap<>(); | ||
|
|
||
| if (itr == null) { | ||
| return files; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd consider at least logging the failure, to make debugging locally easier |
||
| } | ||
|
|
||
| while (itr.hasNext()) { | ||
| File nextFile = new File(itr.next(), repo); | ||
| Path nextPath = nextFile.toPath(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -155,6 +155,11 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Path remoteDirPath = localVCRepositoryUri.getLocalRepositoryPath(localVCBasePath); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (Files.exists(remoteDirPath)) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value Error loading related location Loading
Copilot AutofixAI 10 days ago To fix the problem, user-tainted values like The implementation will be:
Changes are required in
Suggested changeset
2
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCService.java
src/main/java/de/tum/cit/aet/artemis/programming/service/localvc/LocalVCRepositoryUri.java
Outside changed files
Copilot is powered by AI and may make mistakes. Always verify output.
Positive FeedbackNegative Feedback
Refresh and try again.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could this also lead to a race condition if two users try to create it at once? Would both processes try to create it? |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.debug("Local git repo {} at {} already exists – skipping creation", repositorySlug, remoteDirPath); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Files.createDirectories(remoteDirPath); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,10 +8,14 @@ | |
| import static de.tum.cit.aet.artemis.core.connector.AthenaRequestMockProvider.ATHENA_RESTRICTED_MODULE_TEXT_TEST; | ||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| import java.nio.file.Files; | ||
| import java.nio.file.Path; | ||
| import java.time.ZonedDateTime; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import org.apache.commons.io.FileUtils; | ||
| import org.junit.jupiter.api.AfterEach; | ||
| import org.junit.jupiter.api.BeforeEach; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.junit.jupiter.params.ParameterizedTest; | ||
|
|
@@ -33,6 +37,7 @@ | |
| import de.tum.cit.aet.artemis.exercise.domain.InitializationState; | ||
| import de.tum.cit.aet.artemis.exercise.domain.participation.StudentParticipation; | ||
| import de.tum.cit.aet.artemis.exercise.participation.util.ParticipationFactory; | ||
| import de.tum.cit.aet.artemis.exercise.participation.util.ParticipationUtilService; | ||
| import de.tum.cit.aet.artemis.exercise.test_repository.StudentParticipationTestRepository; | ||
| import de.tum.cit.aet.artemis.exercise.util.ExerciseUtilService; | ||
| import de.tum.cit.aet.artemis.modeling.domain.ModelingExercise; | ||
|
|
@@ -42,12 +47,17 @@ | |
| import de.tum.cit.aet.artemis.modeling.util.ModelingExerciseUtilService; | ||
| import de.tum.cit.aet.artemis.programming.domain.ProgrammingExercise; | ||
| import de.tum.cit.aet.artemis.programming.domain.ProgrammingSubmission; | ||
| import de.tum.cit.aet.artemis.programming.domain.RepositoryType; | ||
| import de.tum.cit.aet.artemis.programming.icl.LocalVCLocalCITestService; | ||
| import de.tum.cit.aet.artemis.programming.service.GitService; | ||
| import de.tum.cit.aet.artemis.programming.service.localvc.LocalVCRepositoryUri; | ||
| import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseStudentParticipationTestRepository; | ||
| import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingExerciseTestRepository; | ||
| import de.tum.cit.aet.artemis.programming.test_repository.ProgrammingSubmissionTestRepository; | ||
| import de.tum.cit.aet.artemis.programming.util.LocalRepository; | ||
| import de.tum.cit.aet.artemis.programming.util.ProgrammingExerciseParticipationUtilService; | ||
| import de.tum.cit.aet.artemis.programming.util.ProgrammingExerciseUtilService; | ||
| import de.tum.cit.aet.artemis.programming.util.RepositoryExportTestUtil; | ||
| import de.tum.cit.aet.artemis.text.domain.TextExercise; | ||
| import de.tum.cit.aet.artemis.text.domain.TextSubmission; | ||
| import de.tum.cit.aet.artemis.text.repository.TextExerciseRepository; | ||
|
|
@@ -100,6 +110,15 @@ class AthenaResourceIntegrationTest extends AbstractAthenaTest { | |
| @Autowired | ||
| private LearnerProfileUtilService learnerProfileUtilService; | ||
|
|
||
| @Autowired | ||
| private LocalVCLocalCITestService localVCLocalCITestService; | ||
|
|
||
| @Autowired | ||
| private ProgrammingExerciseStudentParticipationTestRepository programmingExerciseStudentParticipationTestRepository; | ||
|
|
||
| @Autowired | ||
| private ParticipationUtilService participationUtilService; | ||
|
|
||
| private TextExercise textExercise; | ||
|
|
||
| private TextSubmission textSubmission; | ||
|
|
@@ -155,6 +174,11 @@ protected void initTestCase() { | |
| modelingSubmissionRepository.save(modelingSubmission); | ||
| } | ||
|
|
||
| @AfterEach | ||
| void cleanupRepositories() { | ||
| RepositoryExportTestUtil.cleanupTrackedRepositories(); | ||
| } | ||
|
|
||
| @Test | ||
| @WithMockUser(username = TEST_PREFIX + "editor1", roles = "EDITOR") | ||
| void testGetAvailableProgrammingModulesSuccess_EmptyModules() throws Exception { | ||
|
|
@@ -391,13 +415,12 @@ void testRepositoryExportEndpoint(String urlSuffix) throws Exception { | |
| programmingExerciseParticipationUtilService.addSolutionParticipationForProgrammingExercise(programmingExercise); | ||
|
|
||
| // Seed a LocalVC bare repository with content | ||
| var sourceRepo = new LocalRepository(defaultBranch); | ||
| var sourceRepo = RepositoryExportTestUtil.trackRepository(new LocalRepository(defaultBranch)); | ||
| sourceRepo.configureRepos(localVCBasePath, "athenaSrcLocalRepo", "athenaSrcOriginRepo"); | ||
|
|
||
| // Ensure tests repository URI exists on the exercise | ||
| var testsSlug = programmingExercise.getProjectKey().toLowerCase() + "-tests"; | ||
| var testsUri = new LocalVCRepositoryUri(localVCBaseUri, programmingExercise.getProjectKey(), testsSlug); | ||
| programmingExercise.setTestRepositoryUri(testsUri.toString()); | ||
| var testsSlug = programmingExercise.getProjectKey().toLowerCase() + "-" + RepositoryType.TESTS.getName(); | ||
| programmingExercise.setTestRepositoryUri(localVCLocalCITestService.buildLocalVCUri(null, null, programmingExercise.getProjectKey(), testsSlug)); | ||
| programmingExerciseRepository.save(programmingExercise); | ||
|
|
||
| var sourceUri = new LocalVCRepositoryUri(localVCBaseUri, sourceRepo.remoteBareGitRepoFile.toPath()); | ||
|
|
@@ -422,6 +445,55 @@ void testRepositoryExportEndpoint(String urlSuffix) throws Exception { | |
| assertThat(repoFiles).as("export returns exactly one file: README.md").isNotNull().hasSize(1).containsOnlyKeys("README.md").containsEntry("README.md", "Initial commit"); | ||
| } | ||
|
|
||
| @Test | ||
| void testStudentRepositoryExportEndpoint() throws Exception { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think coderabbit's suggestion makes sense here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It makes sense. But also throwing generic exception in tests make sense since we would prefer tests to fail early, and gather as much context possible for the failure. It is a common pattern used in our test repo. |
||
| // Enable Athena for the exercise | ||
| programmingExercise.setFeedbackSuggestionModule(ATHENA_MODULE_PROGRAMMING_TEST); | ||
| programmingExerciseRepository.save(programmingExercise); | ||
|
|
||
| // Create a programming student participation with a submission and result | ||
| var studentLogin = TEST_PREFIX + "student1"; | ||
| var result = participationUtilService.addProgrammingParticipationWithResultForExercise(programmingExercise, studentLogin); | ||
| programmingSubmission = (ProgrammingSubmission) result.getSubmission(); | ||
|
|
||
| // Prepare a LocalVC student repository and wire it to the participation referenced by the submission. | ||
| var projectKey = programmingExercise.getProjectKey(); | ||
| String srcSlug = projectKey.toLowerCase() + "-athena-src"; | ||
| var sourceRepo = RepositoryExportTestUtil.trackRepository(localVCLocalCITestService.createAndConfigureLocalRepository(projectKey, srcSlug)); | ||
| // Remove default seed file (test.txt) from the working copy and commit deletion | ||
| var defaultSeed = sourceRepo.workingCopyGitRepoFile.toPath().resolve("test.txt"); | ||
| if (Files.exists(defaultSeed)) { | ||
| Files.delete(defaultSeed); | ||
| sourceRepo.workingCopyGitRepo.add().addFilepattern(".").call(); | ||
| GitService.commit(sourceRepo.workingCopyGitRepo).setMessage("Remove default seed").call(); | ||
| sourceRepo.workingCopyGitRepo.push().setRemote("origin").call(); | ||
| } | ||
| // Seed a README.md so the export contains the expected file | ||
| Path readme = sourceRepo.workingCopyGitRepoFile.toPath().resolve("README.md"); | ||
| FileUtils.writeStringToFile(readme.toFile(), "Initial commit", java.nio.charset.StandardCharsets.UTF_8); | ||
| sourceRepo.workingCopyGitRepo.add().addFilepattern(".").call(); | ||
| GitService.commit(sourceRepo.workingCopyGitRepo).setMessage("Initial commit").call(); | ||
| sourceRepo.workingCopyGitRepo.push().setRemote("origin").call(); | ||
| var studentRepoSlug = localVCLocalCITestService.getRepositorySlug(projectKey, studentLogin); | ||
| var studentLocalVCRepo = RepositoryExportTestUtil.seedLocalVcBareFrom(localVCLocalCITestService, projectKey, studentRepoSlug, sourceRepo); | ||
|
|
||
| // Persist repository URI on the participation | ||
| var programmingStudentParticipation = programmingExerciseStudentParticipationTestRepository.findById(programmingSubmission.getParticipation().getId()).orElseThrow(); | ||
| programmingStudentParticipation.setRepositoryUri(localVCLocalCITestService.buildLocalVCUri(null, null, projectKey, studentRepoSlug)); | ||
| programmingExerciseStudentParticipationTestRepository.save(programmingStudentParticipation); | ||
|
|
||
| // Call the internal endpoint with valid Athena auth and verify file map | ||
| var authHeaders = new HttpHeaders(); | ||
| authHeaders.add(HttpHeaders.AUTHORIZATION, athenaSecret); | ||
|
|
||
| String json = request.get("/api/athena/internal/programming-exercises/" + programmingExercise.getId() + "/submissions/" + programmingSubmission.getId() + "/repository", | ||
| HttpStatus.OK, String.class, authHeaders); | ||
| Map<String, String> repoFiles = request.getObjectMapper().readValue(json, new TypeReference<Map<String, String>>() { | ||
| }); | ||
| assertThat(repoFiles).as("student export returns exactly one file: README.md").isNotNull().hasSize(1).containsOnlyKeys("README.md").containsEntry("README.md", | ||
| "Initial commit"); | ||
| } | ||
|
|
||
| @ParameterizedTest | ||
| @ValueSource(strings = { "repository/template", "repository/solution", "repository/tests" }) | ||
| void testRepositoryExportEndpointsFailWhenAthenaNotEnabled(String urlSuffix) throws Exception { | ||
|
|
@@ -458,4 +530,6 @@ void testRepositoryExportEndpointsFailWithInvalidRepositoryType(String urlSuffix | |
|
|
||
| request.get("/api/athena/internal/programming-exercises/" + programmingExercise.getId() + "/" + urlSuffix, HttpStatus.NOT_FOUND, Result.class, authHeaders); | ||
| } | ||
|
|
||
| // Removed legacy public endpoint test that expected BAD_REQUEST for invalid repository type | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.