Skip to content
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

Added CheckLibraryVersion class for check library version #142

Merged
merged 7 commits into from
Jun 12, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ dependencies {

compileOnly 'org.projectlombok:lombok:1.18.22'
annotationProcessor 'org.projectlombok:lombok:1.18.22'

implementation 'javax.json:javax.json-api:1.1.4'
implementation 'org.glassfish:javax.json:1.1.4'
}

compileJava.options.encoding = 'UTF-8'
Expand Down
129 changes: 129 additions & 0 deletions src/main/java/org/hyperskill/hstest/checker/CheckLibraryVersion.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
package org.hyperskill.hstest.checker;

import org.hyperskill.hstest.exception.outcomes.UnexpectedError;

import javax.json.Json;
import javax.json.JsonObject;
import java.io.*;
import java.net.HttpURLConnection;
import java.net.URL;
import java.time.LocalDate;
import java.util.stream.Collectors;

/**
* Checks if the current version of the library is the latest one on GitHub releases page of the library.
* If not, throws an exception.
*/
public class CheckLibraryVersion {

private String VERSION_FILE = "src/main/java/org/hyperskill/hstest/resources/version.txt";

private String currentVersion;
private String latestVersion;
private boolean isLatestVersion = true;

public CheckLibraryVersion() {
}

/**
* Checks if the current version of the library is the latest one on GitHub releases page of the library.
* If not, throws an exception.
*/
public void checkVersion() {
LocalDate lastChecked = null;
File lastCheckedFile = new File("LastChecked.txt");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check what this file doesn't put into submission.

if (lastCheckedFile.exists()) {
try {
FileReader fileReader = new FileReader(lastCheckedFile);
BufferedReader reader = new BufferedReader(fileReader);
lastChecked = LocalDate.parse(reader.readLine());
reader.close();
} catch (IOException e) {
throw new RuntimeException(e);
Copy link

@aaaaaa2493 aaaaaa2493 May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this method contains a lot of RuntimeException's? That's not a good way of handling checked exceptions making them unchecked exceptions. At the very least they should be UnexpectedError's. Or just make the method as thorows IOException

}
}

if (lastChecked != null && lastChecked.equals(LocalDate.now())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (lastChecked != null && lastChecked.equals(LocalDate.now())) {
if (LocalDate.now().equals(lastChecked)) {

return;
}

InputStream inputStream = getClass().getClassLoader().getResourceAsStream(VERSION_FILE);
if (inputStream == null) {
throw new RuntimeException("Unable to find hs-test library version file");
}

try {
currentVersion = new BufferedReader(new InputStreamReader(inputStream)).readLine();
} catch (IOException e) {
throw new RuntimeException(e);
}
latestVersion = getLatestHsTestVersionFromGitHub();

if (!currentVersion.equals(latestVersion)) {
if (currentVersion.split("\\.")[0] != latestVersion.split("\\.")[0]) {
throw new RuntimeException(getFeedback());
} else {
isLatestVersion = false;
}
}

lastChecked = LocalDate.now();
try (FileWriter writer = new FileWriter(lastCheckedFile)) {
writer.write(lastChecked.toString());
} catch (IOException e) {
throw new RuntimeException(e);
}
}

/**
* Returns latest version of the library from GitHub releases page of the library.
* @return String latest version of the library
*/
private String getLatestHsTestVersionFromGitHub() {
HttpURLConnection connection = null;
try {
URL url = new URL("https://api.github.com/repos/hyperskill/hs-test/releases/latest");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check timeout

connection = (HttpURLConnection) url.openConnection();
connection.setRequestMethod("GET");
connection.setRequestProperty("Accept", "application/vnd.github+json");

int responseCode = connection.getResponseCode();
if (responseCode != 200) {
throw new UnexpectedError("Error getting latest version of hs-test library from GitHub");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, in case GitHub is down our users can't even test their code. It is not OK. I think, in case we can't check library version, you should assume it's the latest version.

Copy link

@aaaaaa2493 aaaaaa2493 May 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, keep in mind that online checking (from the website) is done in the isolated environment and there are no internet connection. Such way of checking should continue working fine. We should not rely on internet being accessible while checking user code. Could you please tell what is a motivation behind this issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you mean that with online verification it will not be possible to get the current version in GitHub, the latestVersion will be equal to the currentVersion and the user will continue to work with tests.
The relevance of the library to work in images for online verification is not the task of the user, we must maintain the relevance of the library for online verification.

Copy link

@aaaaaa2493 aaaaaa2493 May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@polischuks Yes. But online checking in done without access to the internet (Docker image has no access to he internet) and that is why the check will fail. That's what I mean

}
} catch (IOException e) {
throw new RuntimeException(e);
}

try (BufferedReader in = new BufferedReader(new InputStreamReader(connection.getInputStream()))) {
String response = in.lines().collect(Collectors.joining());
JsonObject jsonObject = Json.createReader(new StringReader(response)).readObject();
return jsonObject.getString("tag_name");
} catch (IOException e) {
throw new UnexpectedError("Error getting latest version of hs-test library from GitHub");
}
}

/**
* Returns feedback for the user if the current version of the library is not the latest one.
* @return String feedback for the user
*/
public String getFeedback() {
return "\nThe installed hs-test version (" + currentVersion + ") is not the latest version (" + latestVersion + "). " +
"Update the library by following the instructions below:\n\n" +
"1. Open your project's dependency file build.gradle.\n" +
"2. Find the hs-test dependency and change its version to the latest one (" + latestVersion + ").\n" +
"3. Sync the dependencies in your development environment or run the following commands in the terminal:\n" +
" For Gradle:\n" +
" gradle clean build --refresh-dependencies\n\n" +
"4. Restart the tests.\n\n";
}

/**
* Returns true if the current version of the library is the latest one.
* @return Boolean true if the current version of the library is the latest one
*/
public Boolean getLatestVersion() {
return isLatestVersion;
}
}
1 change: 1 addition & 0 deletions src/main/java/org/hyperskill/hstest/resources/version.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
10.0.3
6 changes: 6 additions & 0 deletions src/main/java/org/hyperskill/hstest/stage/StageTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.hyperskill.hstest.stage;

import lombok.Getter;
import org.hyperskill.hstest.checker.CheckLibraryVersion;
import org.hyperskill.hstest.common.FileUtils;
import org.hyperskill.hstest.common.ReflectionUtils;
import org.hyperskill.hstest.dynamic.ClassSearcher;
Expand Down Expand Up @@ -138,6 +139,8 @@ private void printTestNum(int num) {

@Test
public final void start() {
CheckLibraryVersion checkLibraryVersion = new CheckLibraryVersion();
checkLibraryVersion.checkVersion();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every executable code of hs-test should be under try-catch block. Including this one. It guarantees that in case something goes wrong, the user would still get some message from testing (yes, it would be Unexpected error but it is better than just crash with exception)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why do we even need to check the library beforehand if we use this information only in case the user failed test? We should check the version only in case of fail as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have two variants to check,
if (!currentVersion.equals(latestVersion)) { if (currentVersion.split("\\.")[0] != latestVersion.split("\\.")[0]) { throw new RuntimeException(getFeedback()); } else { isLatestVersion = false; } }
if (currentVersion.split("\\.")[0] != latestVersion.split("\\.")[0]) { throw new RuntimeException(getFeedback());
if the user's version and Git do not equal in main index X.0.0 user has RuntimeException(getFeedback()); and process stoped,
and light checking when the user has failed on the test case.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What kind of feature should it be for the release of (X+1).0.0 so that every user needs to update to it right now? Every major release added support for major technology or language. For example: support of Go testing, support of Bash testing, support of Spring Boot testing. In case the users already testing their applicarion chances are that they simply don't need new major language or technology supported in hs-test. Because whatever they are testing already supported in hs-test.

int currTest = 0;
boolean needTearDown = false;
try {
Expand Down Expand Up @@ -166,6 +169,9 @@ public final void start() {
if (!result.isCorrect()) {
String fullFeedback = result.getFeedback() + "\n\n"
+ testRun.getTestCase().getFeedback();
if (!checkLibraryVersion.getLatestVersion()) {
fullFeedback += checkLibraryVersion.getFeedback();
}
throw new WrongAnswer(fullFeedback.trim());
}

Expand Down