Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); refactor(tests): Make Integration tests dynamic and Json Snapshot Based. by oreenlivnicode · Pull Request #120 · CycodeLabs/raven · GitHub
Skip to content

refactor(tests): Make Integration tests dynamic and Json Snapshot Based. - #120

Merged
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests
Nov 6, 2023
Merged

refactor(tests): Make Integration tests dynamic and Json Snapshot Based.#120
oreenlivnicode merged 7 commits into
mainfrom
refactor/integration-tests

Conversation

@oreenlivnicode

Copy link
Copy Markdown
Contributor

Created organization - RavenIntegrationTests. It will contain only the repositories we want to test.

Refactor the way we test integrations.
We will have a snapshot json file under tests/integration/structures_json/ of the neo4j structure.
To test all the relationships and node properties as expected, we will convert graph objects to json structure using utilities I've added and compare it to the snapshot object.
It will assert every node/relationship remains the same.

This allows us to scale up our tests, and to test complex scenarios with no effort.
This is an example of snapshot file

{
"nodes": [{
"path": "actions/checkout",
"using": "node20",
"inputs": ["repository", "ref", "token", "ssh-key", "ssh-known-hosts", "ssh-strict", "persist-credentials", "path", "clean", "filter", "sparse-checkout", "sparse-checkout-cone-mode", "fetch-depth", "fetch-tags", "show-progress", "lfs", "submodules", "set-safe-directory", "github-server-url"],
"name": "Checkout",
"is_public": true,
"_id": "d35e7df441120da9624b8c11e36151be",
"url": "https://github.com/actions/checkout/tree/main/action.yml",
"labels": ["CompositeAction"]
},
],
"relationships": [{
"start_node": "83b34c58d9a3220059a67e3671ff97f2",
"type": "ACTION",
"end_node": "d35e7df441120da9624b8c11e36151be"
},
]
}

Change test_node_count to use get_nodes_as_dicts.
Add integration tests for graph structures.
Add integration 1 repo.
Add integration retrival and asserting functions.
Add integration consts.
Update test_node_count - not done.
Add function get_sorted_lists_of_nodes_and_relationships in utils
Add sort to structure object before comparison
Add integration tests for Demo-[1-4]
Remove test_node_count.py
@oreenlivnicode
oreenlivnicode requested review from alex-ilgayev and elad-pticha and removed request for elad-ptichaNovember 5, 2023 16:16

@elad-ptichaelad-pticha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Can we get a docs from this? Let's start documenting things like this under wiki/other solution.

Now, for each request, we require contributors to add tests, and we need that process to be fully understood.

@alex-ilgayevalex-ilgayev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great!
This will be a great improvement for our CI process. Great job!
I left few comments, summarizing them:

  • I suggest leaving test_node_count.py for now until this solution will be more mature and tested. Because otherwise we don't have any integrations tests for PRs. Same for the test_pr.yml workflow.
  • We need to think how adding integration tests would be the minimum code addition as possible. In my mind that would be adding/modifying a JSON, and referencing it in the test_graph_structures.py. I would think how to remove completely the integration_consts.py, and do a single loop that goes over all integration tests.
  • To have clear feature coverage, we need to understand the coverage for each integration test we have. Few lines per test would be great.
  • And finally, as Elad said, we need to provide simple guidance for adding new tests.
    Great job overall!

Comment threadsrc/common/utils.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
Comment threadtests/test_raven.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/test_graph_structures.py Outdated
Comment threadtests/integration/integration_consts.py Outdated
@elad-ptichaelad-pticha changed the title Refactor - Make Integration Tests Dynamic and Json Snapshot Based.refactor(tests): Make Integration tests dynamic and Json Snapshot Based.Nov 6, 2023
Change integration tests conts to contain dicts of configs
Add generic integration test run to test_graph_structures.py
@oreenlivnicode
oreenlivnicodeforce-pushed the refactor/integration-tests branch from 0ab7152 to 36b6e2eCompareNovember 6, 2023 13:47
@oreenlivnicode

Copy link
Copy Markdown
ContributorAuthor

@alex-ilgayev@elad-pticha - Check out new integration_consts.py structure.
The test_graph_structures.py file is set up to have a single, generic test that can be used across various integration tests.

Alternatively, we could break down the configuration dictionaries and include each specific configuration in its corresponding JSON structure file. This way, the configuration for a given test would be retrieved directly from its associated JSON file.

@oreenlivnicode
oreenlivnicode merged commit 4fdf625 into mainNov 6, 2023
@oreenlivnicode
oreenlivnicode deleted the refactor/integration-tests branch November 6, 2023 14:29
Sign up for freeto 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.

3 participants

@oreenlivnicode@alex-ilgayev@elad-pticha