Skip to content

Skip validation #411

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

Merged
merged 7 commits into from
Nov 15, 2023
Merged

Skip validation #411

merged 7 commits into from
Nov 15, 2023

Conversation

InnocentBug
Copy link

@InnocentBug InnocentBug commented Nov 13, 2023

Description

As discussed with @bearmit
A flag that disables partially the tests is being implemented.

At the moment we do not disable tests by default.

There is a text output that let's users know that they can disable the validation if needed.
Some checks are forced even as tests are disabled.

  • upon save
  • and upon node.json

Changes

I changed the logging text to let users know that they can disable the validation.
And marked forced checks as such, letting the user know that they can't be disabled and that if they have issues, they should enable the tests.

Tests

I added a test that checks, that a wrong node is accepted if tests are disabled, but detected if forced.

Known Issues

None.

Notes

If we decide to disable tests by default, we just have to change the default value for cript.API.skip_validation to True.

Checklist

  • My name is on the list of contributors (CONTRIBUTORS.md) in the pull request source branch.
  • I have updated the documentation to reflect my changes.

Copy link

trunk-io bot commented Nov 13, 2023

Merging to develop in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

@InnocentBug InnocentBug requested a review from nh916 November 13, 2023 20:57
@nh916
Copy link
Contributor

nh916 commented Nov 13, 2023

please be sure this targets the develop branch as well

@InnocentBug InnocentBug changed the base branch from main to develop November 13, 2023 21:03
@InnocentBug
Copy link
Author

please be sure this targets the develop branch as well

yeah, done for both. (and branches rebased)

nh916
nh916 previously approved these changes Nov 13, 2023
@nh916
Copy link
Contributor

nh916 commented Nov 13, 2023

please be sure this targets the develop branch as well

yeah, done for both. (and branches rebased)

You are the best!

nh916
nh916 previously approved these changes Nov 13, 2023
@nh916
Copy link
Contributor

nh916 commented Nov 14, 2023

I want to approve this but CI is failing

@InnocentBug
Copy link
Author

I want to approve this but CI is failing

should be fixed.

@@ -183,7 +183,7 @@ def extract_base_url(url):
invalid_schema = {"invalid key": "invalid value", "node": ["Material"]}

# Test should be skipped
assert local_cript_api._is_node_schema_valid(node_json=json.dumps(invalid_schema), is_patch=False) is True
assert local_cript_api._is_node_schema_valid(node_json=json.dumps(invalid_schema), is_patch=False) is None
Copy link
Contributor

Choose a reason for hiding this comment

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

Beautiful!

@nh916 nh916 mentioned this pull request Nov 15, 2023
@InnocentBug InnocentBug merged commit d97eb4f into develop Nov 15, 2023
@InnocentBug InnocentBug deleted the skip-validation branch November 15, 2023 19:50
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