-
Notifications
You must be signed in to change notification settings - Fork 294
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
Programming exercises
: Add typescript programming language template
#9440
Conversation
Programming exercises
: Add TypeScript programming language templateProgramming exercises
: Add typescript programming language template
d7d46a1
to
641c487
Compare
WalkthroughThe changes in this pull request primarily introduce support for TypeScript across various components of the project. This includes adding TypeScript as a supported programming language in configuration files, updating dependency management in Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 57
🧹 Outside diff range comments (1)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Line range hint
1-59
: Summary: TypeScript support successfully added to LocalCIProgrammingLanguageFeatureServiceThe changes in this file effectively introduce TypeScript support to the
LocalCIProgrammingLanguageFeatureService
. The modifications are minimal, focused, and maintain the existing code structure and patterns. The new TypeScript entry is consistent with other language entries and appears to be correctly configured for TypeScript's characteristics.As the number of supported languages grows, consider refactoring this service to use a more scalable approach, such as loading language configurations from a separate configuration file or database. This would make it easier to add or modify language support without changing the service code.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (6)
src/main/resources/config/application.yml
is excluded by!**/*.yml
src/main/resources/templates/aeolus/typescript/default.yaml
is excluded by!**/*.yaml
src/main/resources/templates/typescript/exercise/package-lock.json
is excluded by!**/package-lock.json
src/main/resources/templates/typescript/solution/package-lock.json
is excluded by!**/package-lock.json
src/main/resources/templates/typescript/test/package-lock.json
is excluded by!**/package-lock.json
src/test/resources/config/application.yml
is excluded by!**/*.yml
📒 Files selected for processing (38)
- build.gradle (1 hunks)
- docs/user/exercises/programming-exercise-features.inc (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1 hunks)
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (2 hunks)
- src/main/resources/templates/aeolus/typescript/default.sh (1 hunks)
- src/main/resources/templates/jenkins/typescript/regularRuns/pipeline.groovy (1 hunks)
- src/main/resources/templates/typescript/exercise/.gitignore (1 hunks)
- src/main/resources/templates/typescript/exercise/package.json (1 hunks)
- src/main/resources/templates/typescript/exercise/src/bubblesort.ts (1 hunks)
- src/main/resources/templates/typescript/exercise/src/client.ts (1 hunks)
- src/main/resources/templates/typescript/exercise/src/context.ts (1 hunks)
- src/main/resources/templates/typescript/exercise/src/mergesort.ts (1 hunks)
- src/main/resources/templates/typescript/exercise/src/policy.ts (1 hunks)
- src/main/resources/templates/typescript/exercise/src/sortstrategy.ts (1 hunks)
- src/main/resources/templates/typescript/exercise/tsconfig.json (1 hunks)
- src/main/resources/templates/typescript/readme (1 hunks)
- src/main/resources/templates/typescript/solution/.gitignore (1 hunks)
- src/main/resources/templates/typescript/solution/package.json (1 hunks)
- src/main/resources/templates/typescript/solution/src/bubblesort.ts (1 hunks)
- src/main/resources/templates/typescript/solution/src/client.ts (1 hunks)
- src/main/resources/templates/typescript/solution/src/comparable.ts (1 hunks)
- src/main/resources/templates/typescript/solution/src/context.ts (1 hunks)
- src/main/resources/templates/typescript/solution/src/mergesort.ts (1 hunks)
- src/main/resources/templates/typescript/solution/src/policy.ts (1 hunks)
- src/main/resources/templates/typescript/solution/src/sortstrategy.ts (1 hunks)
- src/main/resources/templates/typescript/solution/tsconfig.json (1 hunks)
- src/main/resources/templates/typescript/test/.gitignore (1 hunks)
- src/main/resources/templates/typescript/test/jest.config.js (1 hunks)
- src/main/resources/templates/typescript/test/package.json (1 hunks)
- src/main/resources/templates/typescript/test/src/behavior.test.ts (1 hunks)
- src/main/resources/templates/typescript/test/src/structural.test.ts (1 hunks)
- src/main/resources/templates/typescript/test/tsconfig.json (1 hunks)
- src/main/webapp/app/entities/programming/programming-exercise.model.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (8)
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Pattern
src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_importssrc/main/webapp/app/entities/programming/programming-exercise.model.ts (1)
📓 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/TemplateUpgradePolicyService.java (3)
Learnt from: magaupp PR: ls1intum/Artemis#9074 File: src/main/java/de/tum/in/www1/artemis/service/programming/TemplateUpgradePolicyService.java:35-35 Timestamp: 2024-10-08T15:35:48.768Z Learning: The `defaultRepositoryUpgradeService` in the `TemplateUpgradePolicyService` is used as a default for all non-JAVA languages, with specialized services added based on testing outcomes if necessary.
Learnt from: magaupp PR: ls1intum/Artemis#9074 File: src/main/java/de/tum/in/www1/artemis/service/programming/TemplateUpgradePolicyService.java:35-35 Timestamp: 2024-07-16T20:00:50.653Z Learning: The `defaultRepositoryUpgradeService` in the `TemplateUpgradePolicyService` is used as a default for all non-JAVA languages, with specialized services added based on testing outcomes if necessary.
Learnt from: magaupp PR: ls1intum/Artemis#9074 File: src/main/java/de/tum/in/www1/artemis/service/programming/TemplateUpgradePolicyService.java:35-35 Timestamp: 2024-10-08T15:35:42.972Z Learning: The `defaultRepositoryUpgradeService` in the `TemplateUpgradePolicyService` is used as a default for all non-JAVA languages, with specialized services added based on testing outcomes if necessary.
src/main/resources/templates/aeolus/typescript/default.sh (2)
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/resources/templates/aeolus/rust/default.sh:14-24 Timestamp: 2024-10-08T15:35:52.595Z Learning: The `default.sh` script in the `src/main/resources/templates/aeolus/rust/` directory is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.
Learnt from: magaupp PR: ls1intum/Artemis#8802 File: src/main/resources/templates/aeolus/rust/default.sh:14-24 Timestamp: 2024-07-01T10:48:58.558Z Learning: The `default.sh` script in the `src/main/resources/templates/aeolus/rust/` directory is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.
🪛 Biome
src/main/resources/templates/typescript/exercise/src/sortstrategy.ts
[error] 1-3: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (61)
src/main/resources/templates/typescript/solution/src/comparable.ts (1)
1-3
: Well-designed TypeScript interface for comparison operations.The
Comparable
interface is a great addition to the TypeScript template. It provides a clear contract for objects that can be compared, which is essential for sorting and comparison operations. The use of TypeScript's interface feature and type annotations aligns well with the PR objectives.Some benefits of this implementation:
- It's exported as default, making it easy to import and use in other files.
- The
valueOf()
method returning a number is a standard approach for comparison, ensuring consistency across implementing classes.- The interface is concise yet effective, following TypeScript best practices.
This interface will be particularly useful when implementing sorting strategies, as mentioned in the PR objectives.
src/main/resources/templates/typescript/test/.gitignore (5)
1-1
: LGTM: Correctly ignoringnode_modules/
.This entry properly excludes the
node_modules
directory from version control, which is a best practice for Node.js and TypeScript projects.
2-2
: LGTM: Correctly ignoring/dist/
.This entry appropriately excludes the
dist
directory, which typically contains compiled output in TypeScript projects.
3-3
: LGTM: Correctly ignoring*.tsbuildinfo
files.This entry properly excludes TypeScript build information files, which are environment-specific and should not be version controlled.
5-5
: Please clarify the purpose of the/assignment
directory.While ignoring this directory may be appropriate, it would be helpful to understand its intended use in the context of the TypeScript template. This will ensure that we're following the best practices for version control in this project.
Could you provide more information about the contents and purpose of the
assignment
directory?
6-6
: Verify ifjunit.xml
is the correct test output format.While ignoring test output files is a good practice, it's worth confirming if
junit.xml
is the expected format for test results in this TypeScript template. Typically, TypeScript projects might use different testing frameworks that produce outputs in other formats (e.g., Jest).Could you confirm that
junit.xml
is indeed the test output format being used in this TypeScript template?src/main/resources/templates/typescript/exercise/src/mergesort.ts (1)
1-3
: LGTM: Class structure is correctly set up.The
MergeSort
class is properly declared and exported as default, which is good for module usage in TypeScript. The class name follows the PascalCase convention, which is appropriate for TypeScript classes.src/main/resources/templates/typescript/exercise/src/context.ts (2)
1-3
: LGTM: Basic class structure is correct.The
Context
class is correctly defined and exported using TypeScript syntax. The use ofexport default
is appropriate for this module.
1-3
: 🧹 Nitpick (assertive)Verify consistency with other language templates.
As this is part of a new TypeScript template for programming exercises, please ensure that the structure and level of implementation in this
Context
class aligns with similar template files for other supported languages (e.g., JavaScript).If the intention is to provide a minimal skeleton for students to implement, this structure is appropriate. However, if other language templates include more detailed comments or partial implementations, consider aligning this TypeScript template accordingly.
Would you like assistance in:
- Comparing this template with other language templates?
- Drafting a more detailed skeleton or comments based on the UML diagram mentioned in the TODO?
Let me know if you need any help with these tasks.
src/main/resources/templates/typescript/exercise/src/policy.ts (1)
1-3
: LGTM: Class structure is correct.The
Policy
class is correctly defined and exported as a default class in TypeScript. This provides a good starting point for students to implement the required functionality.src/main/resources/templates/typescript/solution/src/sortstrategy.ts (2)
1-1
: LGTM: Clean import statementThe import statement is concise and correctly imports the
Comparable
interface from a local file. This promotes good modularity and code organization.
3-5
: Well-structured generic interfaceThe
SortStrategy
interface is well-defined:
- It uses TypeScript's generic syntax correctly with
<T extends Comparable = Date>
.- Extending
Comparable
is appropriate for sorting strategies, ensuring that the generic typeT
has comparison capabilities.- The default type parameter
Date
aligns with the PR objectives.- The
performSort
method signature is correct for a sorting strategy, taking an array of typeT
and returning void.This interface provides a solid foundation for implementing various sorting algorithms in a type-safe manner.
src/main/resources/templates/typescript/exercise/tsconfig.json (1)
1-8
: 🛠️ Refactor suggestionConsider adding more TypeScript-specific compiler options for educational purposes.
While the current configuration is functional, adding more TypeScript-specific options could enhance the learning experience for students using this template.
Consider adding the following compiler options:
{ "extends": "@tsconfig/node20/tsconfig.json", "compilerOptions": { "outDir": "./dist", "rootDir": "./src", "composite": true, + "strictFunctionTypes": true, + "noImplicitReturns": true, + "noFallthroughCasesInSwitch": true, + "forceConsistentCasingInFileNames": true, + "esModuleInterop": true } }These additional options will:
- Enforce stricter function type checking
- Ensure all code paths in a function return a value
- Prevent accidental fallthrough in switch statements
- Enforce consistent file naming
- Improve interoperability between CommonJS and ES Modules
Adding these options can help students learn TypeScript best practices and catch common mistakes early in the development process.
To verify that these options are working as intended, you can run the following script:
This script creates a temporary TypeScript file with intentional issues that should be caught by the suggested compiler options. If the compilation fails, it indicates that the strict options are working as intended.
src/main/resources/templates/typescript/test/jest.config.js (3)
1-1
: LGTM: Appropriate type annotation used.The use of
@type {import('ts-jest').JestConfigWithTsJest}
is a good practice. It ensures type safety for the configuration object and clearly indicates that this is a Jest configuration using ts-jest.
2-7
: LGTM: Correct module exports structure.The use of
module.exports
for exporting the configuration object is correct and follows the expected pattern for Jest configuration files.
3-3
: LGTM: Appropriate test environment set.Setting
testEnvironment: "node"
is correct for a TypeScript project that doesn't require a browser environment. This aligns well with the PR objectives of creating a TypeScript template similar to the existing JavaScript one.src/main/resources/templates/typescript/test/tsconfig.json (1)
1-11
: LGTM! Well-structured TypeScript configuration for testing.The
tsconfig.json
file is well-configured for a TypeScript testing environment:
- It extends the Node.js 20 configuration, ensuring up-to-date TypeScript features.
- The
noEmit
option is correctly set totrue
, which is appropriate for a test configuration.- The project references setup with the "assignment" subdirectory aligns with the PR objectives.
To ensure compatibility, please verify the Node.js version used in the CI/CD pipeline:
This will help confirm that the Node.js version in the CI/CD pipeline matches or exceeds the version specified in the tsconfig (Node.js 20).
✅ Verification successful
LGTM! Node.js version in CI/CD workflows aligns with the TypeScript configuration.
- Node.js 20 is specified in the CI/CD workflow files, ensuring compatibility with the
tsconfig.json
settings.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Node.js version in CI/CD configuration # Test: Look for Node.js version specification in CI config files rg --type yaml -i 'node|nodejs' .github/workflows .gitlab-ci.ymlLength of output: 2168
Script:
#!/bin/bash # Description: Extract the value of env.node in CI/CD configuration files # Search for assignments to env.node rg --type yaml 'env:\s*node\s*:\s*["'\''"]?(\d+)' .github/workflows/Length of output: 70
Script:
#!/bin/bash # Description: Locate the definition of env.node in CI/CD configuration files # Search for where env.node is defined rg --type yaml 'env:\s*node\s*:\s*["'\''"]?(\d+)' .github/workflows/Length of output: 70
src/main/resources/templates/typescript/exercise/package.json (2)
1-3
: LGTM: Project metadata is correctly configured.The project name "artemis-exercise" is appropriate, and marking it as private is correct for an exercise template that's not intended to be published.
8-13
: LGTM: Exports configuration is correctly set up for TypeScript usage.The exports configuration properly defines the locations for TypeScript declaration files and JavaScript files, allowing for correct module resolution in TypeScript projects. The use of wildcards provides flexibility in file naming.
src/main/resources/templates/typescript/solution/package.json (2)
1-3
: LGTM: Project configuration looks good.The project name "artemis-exercise" is appropriate for a template, and marking it as private prevents accidental publication to npm.
1-19
: Overall, excellent package.json configuration for TypeScript template.The package.json file is well-structured and contains all necessary configurations for a TypeScript project. It's appropriate for use as an exercise template in the Artemis system.
Key strengths:
- Correct basic project setup
- Appropriate build and start scripts
- Proper exports configuration for TypeScript
- Relevant dev dependencies
Minor suggestions for improvement have been made in previous comments, but these are not critical issues.
src/main/resources/templates/typescript/test/package.json (2)
1-3
: LGTM: Project metadata is correctly configured.The project name "artemis-test" is appropriate, and marking it as private is a good practice to prevent accidental publishing.
19-23
: LGTM: jest-junit configuration is well-defined.The jest-junit configuration is appropriately set up to generate structured and readable test reports. The custom templates for class names and titles, along with the ancestor separator, will help in parsing and analyzing test results effectively.
src/main/resources/templates/typescript/solution/src/bubblesort.ts (1)
1-2
: LGTM: Import statements are correct and necessary.The import statements for
SortStrategy
andComparable
are appropriate for this implementation.src/main/resources/templates/typescript/solution/src/context.ts (3)
1-1
: LGTM: Correct type import.The import statement correctly uses the
type
keyword for importing theSortStrategy
interface, following TypeScript best practices.
3-3
: LGTM: Class declaration follows conventions.The
Context
class is correctly declared and exported, following TypeScript naming conventions.
1-30
: Overall assessment: Good implementation with room for minor improvements.The
Context
class is well-structured and implements the necessary functionality for managing a sorting algorithm and a collection of dates. The use of TypeScript features like optional chaining and type annotations is commendable. However, there are a few areas where the code could be improved:
- Consider using
readonly
for the_dates
property to prevent accidental reassignment.- Add a return type annotation to the
sort
method for clarity.- Align the
sortAlgorithm
setter parameter type with the property type for consistency.- Replace the
dates
setter with more specific methods for manipulating the dates array.These changes would enhance type safety, clarity, and control over the class's behavior.
src/main/resources/templates/typescript/solution/src/policy.ts (2)
1-5
: LGTM: Imports and constant declaration look good.The import statements are appropriate for the
Policy
class functionality, and theDATES_LENGTH_THRESHOLD
constant follows proper naming conventions.
7-8
: LGTM: Class declaration and constructor are well-structured.The
Policy
class is properly declared with a default export. The constructor uses TypeScript's concise syntax for declaring a private member, which is a good practice.src/main/resources/templates/aeolus/typescript/default.sh (3)
1-3
: LGTM: Script setup and environment variable definition.The script setup follows best practices with a proper shebang and error handling. The
AEOLUS_INITIAL_DIRECTORY
variable is correctly set to store the initial working directory.
33-33
: LGTM: Script execution.The main function is correctly called with all provided arguments, which is the standard practice for Bash scripts.
1-33
: Verify if this file is auto-generated by Aeolus.Based on previous learnings from similar files in other language directories, it's possible that this
default.sh
script is auto-generated by Aeolus. If this is the case, manual edits to this file should be avoided.Could you confirm if this file is auto-generated? If so, any necessary changes should be made in the Aeolus generator settings or templates instead of directly editing this file.
src/main/resources/templates/typescript/test/src/structural.test.ts (6)
1-10
: LGTM: Imports and type assertions are correctly implemented.The imports from the 'artemis-exercise' module and the subsequent type assertions to 'any' are correctly implemented. This approach allows for runtime errors to be captured as test failures, which aligns with the PR objectives.
13-24
: LGTM: Context tests are comprehensive and well-structured.The tests for the Context class adequately verify the presence of required properties ('dates', 'sortAlgorithm') and the 'sort' method. The use of Jest's expect API is correct and follows best practices for structural testing.
26-37
: LGTM: Policy tests are correctly implemented.The tests for the Policy class properly verify the presence of the 'context' property and the 'configure' method. The structure and implementation of these tests are correct and align with the expected behavior of the Policy class.
39-43
: LGTM: BubbleSort test is correctly implemented.The test for the BubbleSort class correctly verifies the presence of the 'performSort' method on the prototype. This approach ensures that the method is shared across all instances of the BubbleSort class, which is an efficient way to define methods in JavaScript/TypeScript.
45-49
: LGTM: MergeSort test is consistent with BubbleSort test.The test for the MergeSort class correctly verifies the presence of the 'performSort' method on the prototype. The structure and implementation are consistent with the BubbleSort test, which is excellent for maintaining code uniformity and readability.
1-50
: Excellent implementation of structural tests for TypeScript classes.This file successfully implements structural tests for the TypeScript classes (Context, Policy, BubbleSort, and MergeSort) as outlined in the PR objectives. The tests are well-structured, consistent, and provide good coverage of the expected properties and methods for each class. The use of type assertions to allow for runtime error checking aligns with the PR's goal of capturing runtime errors as test failures.
A few points to highlight:
- The tests effectively verify the presence of required properties and methods for each class.
- The use of Jest's expect API is consistent and correct throughout the file.
- Testing methods on prototypes ensures shared functionality across instances.
Overall, this implementation successfully supports the introduction of the TypeScript programming exercise template and aligns well with the PR objectives.
src/main/java/de/tum/cit/aet/artemis/programming/domain/ProgrammingLanguage.java (2)
Line range hint
24-24
: LGTM: TypeScript added to ProgrammingLanguage enumThe addition of TYPESCRIPT with the file extension "ts" is correct and consistent with the existing structure of the enum. This change aligns with the PR objective of adding TypeScript support to the project.
Line range hint
1-85
: Overall review: File adheres to coding guidelinesThe
ProgrammingLanguage.java
file follows the provided coding guidelines:
- Uses CamelCase naming conventions
- Adheres to the single responsibility principle
- Avoids code duplication
- Keeps the implementation simple (KISS principle)
- Uses appropriate access modifiers
- Avoids star imports
The changes to add TypeScript support are consistent with the existing structure and the PR objectives. No additional issues or improvements are necessary.
src/main/resources/templates/typescript/exercise/src/client.ts (2)
46-56
: Well-implemented randomDateWithin functionThe
randomDateWithin
function is correctly implemented and efficiently uses thevalueOf
method to work with timestamps. The TypeScript typing is appropriate, making the function signature clear and type-safe.
1-69
: Summary and Next StepsOverall, the TypeScript implementation for date sorting is well-structured. The utility functions for generating random dates and integers are correctly implemented. Here are the key points and next steps:
- Address the unused imports of
Context
andPolicy
.- Implement the TODO items in the
main
function to complete the sorting logic.- Consider parameterizing the date range in
createRandomDates
for better reusability.- Minor improvements can be made to
randomIntegerWithin
for readability.Once these points are addressed, the implementation will be complete and ready for testing.
src/main/resources/templates/typescript/solution/src/client.ts (2)
1-7
: LGTM: Imports and constants are well-defined.The imports are appropriate for the file's functionality, and the use of constants for configuration values enhances maintainability. The naming convention for constants is correctly followed.
49-59
: LGTM: randomDateWithin function is well-implemented.The function effectively generates a random Date within the given range. The use of the randomIntegerWithin helper function promotes code reusability, and the implementation is concise and correct.
src/main/resources/templates/typescript/test/src/behavior.test.ts (3)
1-10
: LGTM: Imports and type casting are correctly implemented.The imports from the 'artemis-exercise' package are appropriate for the test file's purpose. The intentional casting of imported classes to 'any' type (lines 7-10) aligns with the PR objective of allowing runtime errors to be captured as test failures. This approach ensures that incorrect type structures in student implementations will fail with runtime errors, which can then be detected in the tests.
12-30
: LGTM: Test data setup is well-structured.The test data setup is well-organized:
- The 'datesWithCorrectOrder' constant (lines 13-18) provides a reference for the expected sorted order.
- The 'beforeEach' block (lines 22-30) ensures each test starts with a fresh, unsorted 'dates' array.
This structure provides a solid foundation for the sorting algorithm tests.
1-77
: Overall, the test file is well-implemented and aligns with PR objectives.This new TypeScript test file successfully implements comprehensive tests for sorting algorithms (BubbleSort and MergeSort) and the policy-based sorting strategy. The use of 'any' type casting aligns with the PR objective of capturing runtime errors as test failures, which is crucial for evaluating student implementations.
Key strengths:
- Well-structured test data setup
- Consistent implementation across different sorting algorithms
- Effective testing of the Policy class for different list sizes
While the current implementation is solid, consider the minor improvements suggested in the previous comments to enhance test coverage and maintainability.
src/main/resources/templates/typescript/readme (1)
44-76
: Excellent UML diagram representation.The UML diagram effectively illustrates the class structure and relationships described in the exercise. The use of color coding and hide directives enhances readability and focuses attention on the key elements.
src/main/java/de/tum/cit/aet/artemis/programming/service/jenkins/JenkinsProgrammingLanguageFeatureService.java (2)
13-13
: LGTM: Import statement for TypeScript added correctly.The import statement for TypeScript is correctly placed and follows the coding guidelines.
Line range hint
1-51
: Overall: Changes for TypeScript support look good.The modifications to add TypeScript support in this file are minimal, focused, and consistent with the existing code structure. They adhere to the provided coding guidelines, including naming conventions, single responsibility principle, and avoiding star imports.
To ensure full integration:
- Verify that all necessary configurations for TypeScript are in place across the project.
- Update relevant documentation to include information about TypeScript support.
- Ensure that appropriate test cases for TypeScript functionality are added.
- Check if any build scripts or CI/CD pipelines need updates to accommodate TypeScript.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
16-16
: LGTM: Import statement for TypeScript is correctly added.The new import statement for the TYPESCRIPT constant follows the existing pattern and is placed in the correct alphabetical order.
src/main/java/de/tum/cit/aet/artemis/programming/service/ci/ContinuousIntegrationService.java (2)
222-222
: LGTM: TypeScript support added consistentlyThe addition of
TYPESCRIPT
to the list of programming languages returning "assignment" is consistent with the existing pattern for other languages. This change appropriately extends the service's capability to handle TypeScript assignments.
233-233
: LGTM: TypeScript test handling aligned with other languagesThe addition of
TYPESCRIPT
to the list of programming languages returning an empty string for test repository checkout path is consistent with the existing pattern for languages like Java, Python, etc. This change ensures that TypeScript tests will be handled similarly to other languages in this group.docs/user/exercises/programming-exercise-features.inc (2)
Line range hint
1-81
: Overall, excellent addition of TypeScript support to the documentation.The changes made to include TypeScript support in the programming language and feature support tables are well-executed and maintain the consistency of the existing documentation. These updates effectively communicate the new TypeScript support to users, aligning perfectly with the PR objectives.
A few minor suggestions have been made for formatting and verification, but these are not critical. The overall quality of the changes is high, and they successfully integrate TypeScript into the Artemis platform documentation.
80-81
: LGTM! Consider clarifying Jenkins support for auxiliary repositories.The addition of TypeScript to the feature support table is correct and consistent with the existing structure. The feature support aligns well with the PR objectives and the provided summary.
To ensure consistency with other language entries and clarify the Jenkins support for auxiliary repositories, consider running the following script:
If the script confirms consistency, no changes are needed. Otherwise, consider updating the TypeScript entry or other language entries to maintain consistency in the documentation.
✅ Verification successful
Verified: TypeScript entry is consistent with other language entries.
The TypeScript entry in the feature support table adheres to the same "L: yes, J: no" pattern as other languages, ensuring consistency across the documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of auxiliary repositories support across languages # Test: Check if all languages have the same "L: yes, J: no" pattern for auxiliary repositories rg --type rst "^\s*\|.*\|.*\|.*\|.*\|.*\|.*\|.*\|.*\| L: yes, J: no\s*\|$" docs/user/exercises/programming-exercise-features.inc # If the above command returns all language entries, it confirms consistency. # If not, it might indicate a discrepancy that needs to be addressed.Length of output: 3462
src/main/java/de/tum/cit/aet/artemis/plagiarism/service/ProgrammingPlagiarismDetectionService.java (3)
43-43
: LGTM: New import for TypeScript supportThe import statement for
TypeScriptLanguage
is correctly placed and follows the naming convention. This addition is necessary for supporting TypeScript in the plagiarism detection service.
323-324
: LGTM: TypeScript support added to getJPlagProgrammingLanguageThe changes correctly add support for TypeScript:
- A new case for
TYPESCRIPT
returns aTypeScriptLanguage
instance.TYPESCRIPT
has been removed from the default case.These modifications maintain the method's structure and adhere to the single responsibility principle.
43-43
: Summary: TypeScript support successfully addedThe changes in this file successfully add support for TypeScript in the plagiarism detection service:
- A new import for
TypeScriptLanguage
has been added.- The
getJPlagProgrammingLanguage
method has been updated to handle TypeScript.These modifications are minimal, focused, and adhere to the coding guidelines. They effectively implement the PR objective of introducing TypeScript support for programming exercises.
Also applies to: 323-324
build.gradle (1)
259-259
: LGTM: TypeScript dependency added correctly.The new TypeScript dependency for JPlag has been added correctly, consistent with other language dependencies. This aligns with the PR objective of adding TypeScript support.
To ensure the new dependency is correctly integrated, please run the following command:
This will show the resolved version and any potential conflicts with other dependencies.
✅ Verification successful
Verification Successful: TypeScript dependency confirmed.
The TypeScript dependencyde.jplag:typescript:${jplag_version}
has been successfully added tobuild.gradle
on line 259. No issues were found with the addition.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the TypeScript dependency is correctly added and doesn't cause conflicts ./gradlew dependencies | grep -A 5 "de.jplag:typescript"Length of output: 241
Script:
#!/bin/bash # Search for the TypeScript dependency in build.gradle grep 'implementation "de.jplag:typescript:${jplag_version}"' build.gradleLength of output: 133
src/main/resources/templates/typescript/solution/src/mergesort.ts (1)
51-51
: Ensure the 'Comparable' interface supports comparisonThe comparison in line 51 uses the
valueOf()
method:if (input[leftIndex].valueOf() <= input[rightIndex].valueOf())Verify that the
Comparable
interface enforces the implementation of thevalueOf()
method. Alternatively, consider defining acompareTo
method within theComparable
interface for clearer and more robust comparisons.src/main/resources/templates/jenkins/typescript/regularRuns/pipeline.groovy (1)
1-62
: The pipeline script is well-structured and aligns with existing templatesThe configuration correctly sets up the build and test stages for TypeScript exercises, following established patterns in the project.
...a/de/tum/cit/aet/artemis/programming/service/jenkins/build_plan/JenkinsBuildPlanService.java
Outdated
Show resolved
Hide resolved
src/main/resources/templates/typescript/solution/src/mergesort.ts
Outdated
Show resolved
Hide resolved
src/main/resources/templates/typescript/solution/src/mergesort.ts
Outdated
Show resolved
Hide resolved
src/main/resources/templates/jenkins/typescript/regularRuns/pipeline.groovy
Show resolved
Hide resolved
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.
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.
Tested on TS3. It worked good without errors. 10/10 tests passed for the solution repo and 0/10 passed for the template repo.
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.
Code looks good to me 👍
Checklist
General
Server
Client
Changes affecting Programming Exercises
Motivation and Context
Description
This PR adds a TypeScript programming exercise template. The exercise is very similar to the JavaScript template.
The test project builds the exercise as a composite with
tsc
and runs the tests withts-jest
.Compared to JS, this exercise adds type annotations, the SortStrategy as a interface and Generics.
The tests cast the student implementations to
any
, which prevents build failures. The resulting runtime errors produce proper test failures.The JavaScript Docker image is reused without changes.
Steps for Testing
Prerequisites:
TypeScript
as the programming languageTemplate
andSolution
build jobs finishTemplate Result
should show 0 out of 10 passed testsSolution Result
should show 10 out of 10 passed testsTestserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Code Review
Manual Tests
Test Coverage
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation