Skip to content

[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498

Closed
sekikn wants to merge 1 commit into
apache:masterfrom
sekikn:AIRFLOW-2612
Closed

[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom
sekikn:AIRFLOW-2612

Conversation

@sekikn

@sekiknsekikn commented Jun 14, 2018

Copy link
Copy Markdown
Contributor

Make sure you have checked all steps below.

JIRA

  • My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description

  • Here are some details about my PR, including screenshots of any UI changes:

tests/operators/hive_operator.py has test cases
called HiveServer2Test and HivePrestoTest,
which are disabled by default.
This PR enables the former as the first step
to improve test coverage for Hive-related
hooks and operators.

HiveServer2Test had the following tests:

  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

This PR enables the tests described above.

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.

Code Quality

  • Passes git diff upstream/master -u -- "*.py" | flake8 --diff

@codecov-io

codecov-io commented Jun 14, 2018•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3498 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.

@Fokko

Fokko commented Jun 15, 2018

Copy link
Copy Markdown
Contributor

@gglanzani PTAL

@gglanzani

gglanzani commented Jun 15, 2018

Copy link
Copy Markdown
Contributor

Hi @sekikn,

I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).

Let's wait until that is merge and then take a look at what can be kept from this.

/cc @Fokko

@sekikn

sekikn commented Jun 15, 2018

Copy link
Copy Markdown
ContributorAuthor

@gglanzani Sure thing! I missed that PR.

@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn

sekikn commented Jun 17, 2018

Copy link
Copy Markdown
ContributorAuthor

@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!
So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.

@gglanzani

gglanzani commented Jun 18, 2018

Copy link
Copy Markdown
Contributor

@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.

The tests that have been removed, moreover, were all mocking the real thing.

And the multiple statements test is a welcome addition

aliceabe pushed a commit to aliceabe/incubator-airflow that referenced this pull request Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani
, '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" + '
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani

, '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('^' + ".*" + '
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani

, '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('^' + ".*" + '
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani

, '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" + '
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani

, '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('^' + ".*" + '
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani

, '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('^' + ".*" + '
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani

, '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); }
})();
})();
[AIRFLOW-2612] Enable some Hive-related tests on CI by sekikn · Pull Request #3498 · apache/airflow · GitHub
Skip to content
[AIRFLOW-2612] Enable some Hive-related tests on CI - #3498
Closed
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Closed
[AIRFLOW-2612] Enable some Hive-related tests on CI#3498
sekikn wants to merge 1 commit into
apache:masterfrom 
sekikn:AIRFLOW-2612
Conversation
@sekikn
@sekiknsekikn
commented
Jun 14, 2018

Copy link


Copy Markdown

Contributor
Make sure you have checked all steps below.
JIRA
  •  My PR addresses the following Airflow JIRA issues and references them in the PR title. For example, "[AIRFLOW-XXX] My Airflow PR"

Description
  •  Here are some details about my PR, including screenshots of any UI changes:
tests/operators/hive_operator.py has test cases

called HiveServer2Test and HivePrestoTest,

which are disabled by default.

This PR enables the former as the first step

to improve test coverage for Hive-related

hooks and operators.
HiveServer2Test had the following tests:
  • test_select_conn, test_multi_statements, test_to_csv, test_select_conn_with_schema, test_get_results_with_schema, test_get_records_with_schema, test_get_pandas_df_with_schema: moved to TestHiveServer2Hook in tests/hooks/test_hive_hook.py since they were tests for HiveServer2Hook
  • test_get_metastore_databases: removed since it was a test for HiveMetastoreHook and TestHiveMetastoreHook in tests/hooks/test_hive_hook.py already has a similar test called test_get_partitions
  • connect_mock: removed since it was unused
Tests
  •  My PR adds the following unit tests OR does not need testing for this extremely good reason:
This PR enables the tests described above.
Commits
  •  My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":

    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"
Documentation
  •  In case of new functionality, my PR adds documentation that describes how to use it.

    • When adding new operators/hooks/sensors, the autoclass documentation generation needs to be added.
Code Quality
  •  Passes git diff upstream/master -u -- "*.py" | flake8 --diff
@codecov-io
codecov-io
commented
Jun 14, 2018•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Merging #3498 into master will not change coverage.

The diff coverage is n/a.
Impacted file tree graph
@@ Coverage Diff @@## master #3498 +/- ##
=======================================
Coverage 77.34% 77.34% =======================================
Files 204 204 Lines 15159 15159 =======================================
Hits 11725 11725 Misses 3434 3434
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data

Powered by Codecov. Last update b9cb54f...17a3839. Read the comment docs.
@Fokko
Fokko
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
@gglanzani PTAL
@gglanzani
gglanzani
commented
Jun 15, 2018

Copy link


Copy Markdown

Contributor
Hi @sekikn,
I am against merging this now. PR #3432 adds lots of tests, and allows tests to run on Python 2 (plus it doesn't mock them).
Let's wait until that is merge and then take a look at what can be kept from this.
/cc @Fokko
@sekikn
sekikn
commented
Jun 15, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani Sure thing! I missed that PR.
@sekikn
[AIRFLOW-2612] Clean up Hive-related tests
17a3839
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
@sekikn
sekikn
commented
Jun 17, 2018

Copy link


Copy Markdown

ContributorAuthor
@gglanzani I found that #3432 covers almost all of tests implemented in tests.operators.hive_operator:HiveServer2Test. Great work!

So I just moved test_multi_statements to tests.hooks.test_hive_hook:TestHiveServer2Hook and removed others.
@gglanzani
gglanzani
commented
Jun 18, 2018

Copy link


Copy Markdown

Contributor
@Fokko +1 on this one. All the tests that were mocked are now in #3432 indeed.
The tests that have been removed, moreover, were all mocking the real thing.
And the multiple statements test is a welcome addition
aliceabe pushed a commit
to aliceabe/incubator-airflow
that referenced
this pull request
Jan 3, 2019
@sekiknAlice Berard
[AIRFLOW-2612][AIRFLOW-2534] Clean up Hive-related tests
165186d
There is several HiveServer2-related tests
which are not run by default in
tests/operators/hive_operator.py,
but AIRFLOW-2534 implemented almost all of them
in airflow/hooks/hive_hooks.py.
This PR removes duplicated tests from
tests/operators/hive_operator.py and
moves an unimplemented test to
airflow/hooks/hive_hooks.py with a bit of fix.
Closesapache#3498 from sekikn/AIRFLOW-2612
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers


No reviews


Assignees


No one assigned


Labels


None yet


Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

4 participants

@sekikn@codecov-io@Fokko@gglanzani