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

WIP Implement SfTest and sinceVersion #1583

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

WIP Implement SfTest and sinceVersion #1583

wants to merge 1 commit into from

Conversation

sfc-gh-pfus
Copy link
Contributor

Overview

SNOW-XXXXX

External contributors - please answer these questions before submitting a pull request. Thanks!

Please answer these questions before submitting your pull requests. Thanks!

  1. What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

    Fixes #NNNN

  2. Fill out the following pre-review checklist:

    • I am adding a new automated test(s) to verify correctness of my new code
    • I am adding new logging messages
    • I am modifying authorization mechanisms
    • I am adding new credentials
    • I am modifying OCSP code
    • I am adding a new dependency
  3. Please describe how your code solves the related issue.

    Please write a short description of how your code change solves the related issue.

Pre-review checklist

  • This change has passed precommit
  • I have reviewed code coverage report for my PR in (Sonarqube)


@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.TYPE})
public @interface SfTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

annotation could be put on class, not only method - is this class aware of it?

are we able to handle different version on both levels? e.g. class declares availability since 3.14.5 whereas method declares availability for version 3.14.6

another question is that during implemntation we are not sure which version will be released so the given version should be last version that does not have the tested feature

@Override
public Statement apply(Statement statement, FrameworkMethod frameworkMethod, Object o) {
SfTest sfTest = frameworkMethod.getAnnotation(SfTest.class);
if (sfTest == null || sfTest.sinceVersion() == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

version parameter should be mandatory - if not why don't use normal test paramter?

this.patch = patch;
}

private static Version parse(String str) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

it could be secondary constructor

return statement;
}
Version testSinceVersion = Version.parse(sfTest.sinceVersion());
if (currentVersion.compareTo(testSinceVersion) >= 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

current version is last released version so it should not be equal to 0

Copy link

SonarQube Quality Gate

Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link

github-actions bot commented Jan 26, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants