Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow global config for rule testcases #1580

Merged
merged 15 commits into from Oct 12, 2021

Conversation

sti0
Copy link
Contributor

@sti0 sti0 commented Oct 9, 2021

Brief summary of the change made

This allows to define a global config (e.g. dialect) per rule test. If a config is defined within a single testcase, this one still respected (behaviour as is).

Example:

rule: R001
configs:
    core:
      dialect: exasol


tc1:
  pass_str: |
    create table if not exists tab.xxx (col1 varchar(10))
  configs:
    core:
      dialect: ansi

tc2:
  pass_str: |
    create table tab.xxx (col1 varchar(10))

tc3:
  pass_str: |
    create or replace table tab.xxx (col1 varchar(10))

While tc1 still uses ansi dialect, tc2 and tc3 are using exasol dialect

...

Are there any other side effects of this change that we should be aware of?

...

Pull Request checklist

  • Please confirm you have completed any of the necessary steps below.

  • Included test cases to demonstrate any code changes, which may be one or more of the following:

    • .yml rule test cases in test/fixtures/rules/std_rule_cases.
    • .sql/.yml parser test cases in test/fixtures/parser (note YML files can be auto generated with python test/generate_parse_fixture_yml.py or by running tox locally).
    • Full autofix test cases in test/fixtures/linter/autofix.
    • Other.
  • Added appropriate documentation for the change.

  • Created GitHub issues for any relevant followup/future enhancements if appropriate.

@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #1580 (8290b23) into main (a475335) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #1580      +/-   ##
===========================================
+ Coverage   99.98%   100.00%   +0.01%     
===========================================
  Files         131       131              
  Lines        9175      9180       +5     
===========================================
+ Hits         9174      9180       +6     
+ Misses          1         0       -1     
Impacted Files Coverage Δ
src/sqlfluff/testing/rules.py 100.00% <100.00%> (ø)
src/sqlfluff/core/config.py 100.00% <0.00%> (+0.35%) ⬆️

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 a475335...8290b23. Read the comment docs.

Copy link
Member

@alanmcruickshank alanmcruickshank left a comment

Choose a reason for hiding this comment

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

This looks mostly great. I'd love to add a little more explanation around the test case that you've added, but otherwise excellent. Thanks for putting this together 🚀 .

test/rules/yaml_test_cases_test.py Outdated Show resolved Hide resolved
@tunetheweb
Copy link
Member

What’s the use case out of interest? Is this more for custom rules?

@sti0
Copy link
Contributor Author

sti0 commented Oct 10, 2021

What’s the use case out of interest? Is this more for custom rules?

Exactly. We only run Exasol database and all of the custom rules should be tested against this dialect.

@tunetheweb
Copy link
Member

@alanmcruickshank looks like you need to re-review before this can be merged.

@alanmcruickshank alanmcruickshank merged commit 64ab797 into sqlfluff:main Oct 12, 2021
ttomasz pushed a commit to ttomasz/sqlfluff that referenced this pull request Oct 12, 2021
* FIx broken block comments in exasol

* Allow adding a global test config per testcase

* Respect config per testcase

* Allow adding a global test config per testcase

* Respect config per testcase

* add testcase

* make unittest description more meaningful

Co-authored-by: Alan Cruickshank <alanmcruickshank@gmail.com>
Co-authored-by: Barry Pollard <barry@tunetheweb.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants