Merge pull request #190 from looker-open-source/manifest-features

Manifest features

README.md

@@ -30,7 +30,7 @@ Interested? See a video of LAMS in action!
 
 The linter comes with built-in rules that can enforce rules K1-4, F1-4, E1-2, T1-2, and W1 from the [style guide](https://looker-open-source.github.io/look-at-me-sideways/rules.html).
 
-As of LAMS v3, you must opt-in via your `manifest.lkml` file to use the built-in rules. Here is an example declaration opting in to all the currently available built-in rules:
+As of LAMS v3, you must opt-in via your [manifest](#manifest-configurations) to use the built-in rules. Here is an example declaration opting in to all the currently available built-in rules:
 
 ```lkml
 #LAMS
@@ -50,7 +50,6 @@ As of LAMS v3, you must opt-in via your `manifest.lkml` file to use the built-in
 #rule: T1{} # Triggers use datagroups 
 #rule: T2{} # Primary keys in DT
 #rule: W1{} # Block indentation
-#rule: W1{} # Block indentation
 ```
 
 ### Custom Rules
@@ -80,11 +79,8 @@ view: rollup {
 ```js
 {"rule":"K3","location":"model:my_model/view:rollup"}
 {"rule":"K3","location":"model:my_other_model/view:foo"}
-
 ```
 
-You may also apply rule_exemptions globally in your project.manifest, but this is generally unnecessary as of LAMS v3.
-
 ### Output
 
 Once LAMS has evaluated your project against the necessary rules, the resulting list of messages are communicated back to you through one of several output modes.
@@ -111,9 +107,9 @@ cd <your-lookml-project>
 lams
 ```
 
-- **[Github Action](https://looker-open-source.github.io/look-at-me-sideways/github-action)** - This option is very quick to get started if you're using Github, and offers a compromise between convenience of setup and per-commit run performance.
+- **[Github Action](https://looker-open-source.github.io/look-at-me-sideways/github-action)** - This option is commonly used among LAMS users, and has an up-to-date and convenient deployment example.
 
-The following examples were prepared for v1 of LAMS, though updating them for v2+ should be straightforward. Please review [v2 release notes](https://looker-open-source.github.io/look-at-me-sideways/release-notes/v2) for details. In particular, look for error messages on the console's standard output rather than a file output to be committed back to the repo. 
+The remaining examples were prepared for v1 of LAMS, though updating them for v2+ should be straightforward. Please review [v2 release notes](https://looker-open-source.github.io/look-at-me-sideways/release-notes/v2) for details. In particular, look for error messages on the console's standard output rather than a file output to be committed back to the repo. 
 
 - **[GitLab CI](https://looker-open-source.github.io/look-at-me-sideways/gitlab-ci)** - A community-contributed configuration for GitLab, which offers similarly low overhead as our dockerized Jenkins configuration
 - **[Dockerized Jenkins Server](https://github.com/looker-open-source/look-at-me-sideways/blob/master/docker/README.md)** - We have provided a Docker image with an end-to-end configuration including a Jenkins server, LAMS, and Github protected branches & status checks configuration. 
@@ -130,23 +126,34 @@ The following examples were prepared for v1 of LAMS, though updating them for v2
 - **source** - A glob specifying which files to read. Defaults to `**/{*.model,*.explore,*.view,manifest}.lkml`.
 - **cwd** - A path for LAMS to use as its current working directory. Useful if you are not invoking lams from your LookML repo directory.
 - **project-name** - An optional name for the project, used to generate links back to the project in mardown output. Specifying this in manifest.lkml is preferred.
-- **manifest** - A JSON-encoded object to override any properties that are normally set via the manifest.lkml file.
+- **manifest-defaults** - A file path or JSON-encoded object of default manifest values which may be overriden by the project manifest. The target file may be a lkml file, JSON file, or a YAML file if the `js-yaml` optional peer dependency is installed.
+- **manifest** - A file path or JSON-encoded object of manifest values that override values set by the project manifest. The target file may be a lkml file, JSON file, or a YAML file if the `js-yaml` optional peer dependency is installed.
 - **on-parser-error** - Set to "info" to indicate that LookML parsing errors should not fail the linter, but yield an `info` level message instead (not all output modes display `info` level messages)
 - **verbose** - Set to also output `verbose` level messages, for output modes that support it (`lines`)
 - **date-output** - Set to "none" to skip printing the date at the top of the `issues.md` file.
 - **allow-custom-rules** - Experimental and not recommended. Used to approve the running of **Javascript-based** custom rules. DO NOT USE TO RUN UNTRUSTED CODE. See [custom rules](https://looker-open-source.github.io/look-at-me-sideways/customizing-lams) for details.
 
-### Manifest.lkml arguments
+### Manifest configuration
+
+More complex configurations, such as [custom rule definitions](https://looker-open-source.github.io/look-at-me-sideways/customizing-lams), are provided in a manifest file. These may be provided either directly in your project's native `manifest.lkml` file using `#LAMS` conditional comments, or in a separate file specified by the `manifest` parameter.
+
+If you have a small number of declarations, the native `manifest.lkml` file is a convenient place to add them.
+
+If you are managing many declarations, you may want to consider installing `js-yaml` to maintain manifest configurations in a separate YAML file, to benefit from improved legibility and syntax highlighting. In this case, provide the path to the file in the `manifest` command-line argument. 
+
+In case both files are provided, declarations from both sources will be used, with the latter taking precedence. Similarly, the `manifest-defaults` command-line argument can be used to provide declarations with a lower priority than the native `manifest.lkml`.
 
-If your LookML project doesn't have a manifest.lkml file, you may want to consider adding one! LAMS uses the following information from your project's mainfest.lkml file:
+The manifest can provide the following declarations:
 
 - **name** - Recommended. A name for the project, used to generate links back to the project in mardown output. If the native LookML validator complains about an unnecessary project name, you can use a conditional #LAMS comment to specify it.
 - **rule: rule_name** - Recommended. Used to opt-in to built-in rules and to specify custom rules.  See [customizing LAMS](https://looker-open-source.github.io/look-at-me-sideways/customizing-lams)
-- **rule_exemptions** - Optional. Originally used in 1 & v2 to opt-out of rules globally. A global opt-out can still be useful for opting-out of certain "sub rules" that a rule may return without opting-out of the entire rule.  See [customizing LAMS](https://looker-open-source.github.io/look-at-me-sideways/customizing-lams)
+- **rule_exemptions** - Optional. Originally used in v1 & v2 to opt-out of rules globally. A global opt-out can still be useful for opting-out of certain "sub rules" that a rule may return without opting-out of the entire rule.
 
 ### Optional Dependencies
 
-- `js-yaml` is not automatically installed with LAMS, but you may explicitly install it if you want to write custom rules that lint against the contents of a LookML Dashboard, which is a YAML-based file. In this case, also make sure to pass a `source` argument, as the default `source` does not includes `.dashboard.lookml` files.
+- `js-yaml` is not automatically installed with LAMS, but you may explicitly install it:
+  - if you want to maintain your manifest configuration in YAML
+  - if you want to write custom rules that lint against the contents of a LookML Dashboard, which is a YAML-based file. In this case, also make sure to pass a `source` argument, as the default `source` does not includes `.dashboard.lookml` files.
 
 ## About
 

__tests__/dummy-projects/29-manifest-loading/defaults/index.test.js

@@ -0,0 +1,68 @@
+let {testName, lams, options, mocks} = require('../../../../lib/test-commons.js')(__dirname,{dirnameOffset:-2})
+
+options = {...options, manifestDefaults:"./manifest-defaults.yaml"}
+
+//CONTINUE HERE
+
+describe('Projects', () => {
+	describe(testName, () => {
+		let {spies, process, console} = mocks()
+		let messages
+		beforeAll( async () => {
+			messages = await lams(options,{process, console});
+		})
+		it("should not error out", ()=> {
+			expect(console.error).not.toHaveBeenCalled()
+		});
+		it("it should not contain any unexpected parser (P0) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P0",
+				level: "error"
+			});
+		});
+		it("it should not contain any parser syntax (P1) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P1",
+				level: "error"
+			});
+		});
+
+		it("no_override should provide correct aggregate info (1 match, 0 exempt, 1 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "no_override",
+				level: "info",
+				description: "Rule no_override summary: 1 matches, 0 matches exempt, and 1 errors"
+			});
+		});
+
+		it("it should error on no_override", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "no_override",
+				level: "error"
+			});
+		});
+
+		it("it should error on partial_override (due to incomplete rule def)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "partial_override",
+				level: "error"
+			});
+		});
+
+
+		it("full_override should provide correct aggregate info (1 match, 0 exempt, 0 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "full_override",
+				level: "info",
+				description: "Rule full_override summary: 1 matches, 0 matches exempt, and 0 errors"
+			});
+		});
+
+		it("it should not error on full_override", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "full_override",
+				level: "error"
+			});
+		});
+	});
+});

__tests__/dummy-projects/29-manifest-loading/defaults/manifest-defaults.yaml

@@ -0,0 +1,10 @@
+rule:
+  no_override:
+    match: "$"
+    expr_rule: "false"
+  partial_override:
+    match: "$"
+    expr_rule: "false"
+  full_override:
+    match: "$"
+    expr_rule: "false"

__tests__/dummy-projects/29-manifest-loading/defaults/manifest.lkml

@@ -0,0 +1,8 @@
+# LAMS
+# rule: partial_override { 
+#	expr_rule: true ;;
+# }
+# rule: full_override {
+#	match: "$"
+#	expr_rule: true ;;
+#}

__tests__/dummy-projects/29-manifest-loading/inline-json/index.test.js

@@ -0,0 +1,61 @@
+let {testName, lams, options, mocks} = require('../../../../lib/test-commons.js')(__dirname,{dirnameOffset:-2})
+
+options = {...options, manifest:`{"rule":{"bad":{
+	"match":"$",
+	"expr_rule": "false"
+}}}`}
+
+describe('Projects', () => {
+	describe(testName, () => {
+		let {spies, process, console} = mocks()
+		let messages
+		beforeAll( async () => {
+			messages = await lams(options,{process, console});
+		})
+		it("should not error out", ()=> {
+			expect(console.error).not.toHaveBeenCalled()
+		});
+		it("it should not contain any unexpected parser (P0) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P0",
+				level: "error"
+			});
+		});
+		it("it should not contain any parser syntax (P1) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P1",
+				level: "error"
+			});
+		});
+
+		it("`bad` should provide correct aggregate info (1 match, 0 exempt, 1 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "bad",
+				level: "info",
+				description: "Rule bad summary: 1 matches, 0 matches exempt, and 1 errors"
+			});
+		});
+
+		it("it should error on bad", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "bad",
+				level: "error"
+			});
+		});
+
+		it("`ok` should provide correct aggregate info (1 match, 0 exempt, 0 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "ok",
+				level: "info",
+				description: "Rule ok summary: 1 matches, 0 matches exempt, and 0 errors"
+			});
+		});
+
+		it("it should not error on ok", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "ok",
+				level: "error"
+			});
+		});
+	});
+});

__tests__/dummy-projects/29-manifest-loading/inline-json/manifest.lkml

@@ -0,0 +1,9 @@
+# LAMS
+# rule: ok { 
+#	match: "$"	
+#	expr_rule: true ;;
+# }
+# rule: bad {
+#	match: "$"
+#	expr_rule: true ;;
+#}

__tests__/dummy-projects/29-manifest-loading/lkml-path/index.test.js

@@ -0,0 +1,58 @@
+let {testName, lams, options, mocks} = require('../../../../lib/test-commons.js')(__dirname,{dirnameOffset:-2})
+
+options = {...options, manifest:`./manifest.lams-lkml`}
+
+describe('Projects', () => {
+	describe(testName, () => {
+		let {spies, process, console} = mocks()
+		let messages
+		beforeAll( async () => {
+			messages = await lams(options,{process, console});
+		})
+		it("should not error out", ()=> {
+			expect(console.error).not.toHaveBeenCalled()
+		});
+		it("it should not contain any unexpected parser (P0) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P0",
+				level: "error"
+			});
+		});
+		it("it should not contain any parser syntax (P1) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P1",
+				level: "error"
+			});
+		});
+
+		it("`bad` should provide correct aggregate info (1 match, 0 exempt, 1 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "bad",
+				level: "info",
+				description: "Rule bad summary: 1 matches, 0 matches exempt, and 1 errors"
+			});
+		});
+
+		it("it should error on bad", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "bad",
+				level: "error"
+			});
+		});
+
+		it("`ok` should provide correct aggregate info (1 match, 0 exempt, 0 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "ok",
+				level: "info",
+				description: "Rule ok summary: 1 matches, 0 matches exempt, and 0 errors"
+			});
+		});
+
+		it("it should not error on ok", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "ok",
+				level: "error"
+			});
+		});
+	});
+});

__tests__/dummy-projects/29-manifest-loading/lkml-path/manifest.lams-lkml

@@ -0,0 +1,4 @@
+rule: bad {
+	match: "$"
+	expr_rule: false;;
+}

__tests__/dummy-projects/29-manifest-loading/lkml-path/manifest.lkml

@@ -0,0 +1,9 @@
+# LAMS
+# rule: ok { 
+#	match: "$"	
+#	expr_rule: true ;;
+# }
+# rule: bad {
+#	match: "$"
+#	expr_rule: true ;;
+#}

__tests__/dummy-projects/29-manifest-loading/yaml-path/index.test.js

@@ -0,0 +1,58 @@
+let {testName, lams, options, mocks} = require('../../../../lib/test-commons.js')(__dirname,{dirnameOffset:-2})
+
+options = {...options, manifest:`./lams-manifest.yml`}
+
+describe('Projects', () => {
+	describe(testName, () => {
+		let {spies, process, console} = mocks()
+		let messages
+		beforeAll( async () => {
+			messages = await lams(options,{process, console});
+		})
+		it("should not error out", ()=> {
+			expect(console.error).not.toHaveBeenCalled()
+		});
+		it("it should not contain any unexpected parser (P0) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P0",
+				level: "error"
+			});
+		});
+		it("it should not contain any parser syntax (P1) errors", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "P1",
+				level: "error"
+			});
+		});
+
+		it("`bad` should provide correct aggregate info (1 match, 0 exempt, 1 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "bad",
+				level: "info",
+				description: "Rule bad summary: 1 matches, 0 matches exempt, and 1 errors"
+			});
+		});
+
+		it("it should error on bad", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "bad",
+				level: "error"
+			});
+		});
+
+		it("`ok` should provide correct aggregate info (1 match, 0 exempt, 0 error)", ()=> {
+			expect({messages}).toContainMessage({
+				rule: "ok",
+				level: "info",
+				description: "Rule ok summary: 1 matches, 0 matches exempt, and 0 errors"
+			});
+		});
+
+		it("it should not error on ok", ()=> {
+			expect({messages}).not.toContainMessage({
+				rule: "ok",
+				level: "error"
+			});
+		});
+	});
+});

__tests__/dummy-projects/29-manifest-loading/yaml-path/lams-manifest.yml

@@ -0,0 +1,4 @@
+rule:
+  bad:
+    match: "$"
+    expr_rule: "false"

__tests__/dummy-projects/29-manifest-loading/yaml-path/manifest.lkml

@@ -0,0 +1,9 @@
+# LAMS
+# rule: ok { 
+#	match: "$"	
+#	expr_rule: true ;;
+# }
+# rule: bad {
+#	match: "$"
+#	expr_rule: true ;;
+#}