Add field to allow skipping of tests by ycombinator · Pull Request #105 · elastic/package-spec

ycombinator · 2021-01-12T20:08:57Z

This PR adds a field to the pipeline and system test configuration files, allowing tests to be skipped.

Related: elastic/elastic-package#218

elasticmachine · 2021-01-12T20:11:23Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Build Cause: Pull request #105 updated
- Start Time: 2021-01-14T21:50:07.628+0000
Duration: 3 min 6 sec
Commit: d77a22e

mtojek

I'm not in favor of skipping tests, but we may need it in the future. I would enforce the description and link with regexp :) Something like these (all fields required):

skip:
   url: 
   explanation:

WDYT?

ycombinator · 2021-01-13T14:52:25Z

@mtojek thanks, I like this idea. It forces users to offer a short explanation for the skip and URL that has more details. 👍

ycombinator · 2021-01-13T20:22:19Z

@mtojek I addressed your review feedback. A few points to note:

I went with link instead of url because we already have a precedent for link here:

package-spec/versions/1/changelog.spec.yml

Line 37 in 930b447

link:
I went with reason instead of explanation because reason is shorter. 😄
I don't like that the definition of skip is repeated in two places. I tried refactoring it into a common file, similar to what we have with

package-spec/versions/1/data_stream/manifest.spec.yml

Line 35 in 930b447

vars:

but I was not successful. I kept getting a Reference /spec.yml#/definitions/skip must be canonical error when I ran make test (after running make update).
- The value of my $ref property looked like "../spec.yml/#definitions/skip" and in the data_stream/_dev/test/spec.yml file I had a spec.definitions.skip property.
- Just for testing I even tried to create a common.yml file in the same folder as the referencing spec file and changed the value of $ref to "./common.yml/#definitions/skip" but that also gave the same error.
- If you have some ideas as to what might be causing this error or what else I should try, please let me know.

mtojek · 2021-01-14T10:41:59Z

I looked into this problem and found a workaround: https://github.com/mtojek/package-spec/tree/test-skip-fix-1

Most likely the inline content doesn't allow for jumping over remote resources or the location is not evaluated correctly.

ycombinator · 2021-01-14T21:28:45Z

Thanks @mtojek. I merged your workaround into this PR. Now I'm seeing a different test failure, which I'm investigating:

=== RUN   TestValidateFile/good
    validator_test.go:50:
                Error Trace:    validator_test.go:50
                Error:          Received unexpected error:
                                found 1 validation error:
                                   1. item [test-access-event.json-config.json] is not allowed in folder [../../internal/validator/test/packages/good/data_stream/foo/_dev/test/pipeline]
                Test:           TestValidateFile/good

ycombinator · 2021-01-14T22:18:07Z

@mtojek This PR is ready for your re-review when you get a chance. Thanks!

mtojek · 2021-01-14T23:27:19Z

versions/1/data_stream/_dev/test/pipeline/config_raw.spec.yml

@@ -0,0 +1,20 @@
+spec:


I think the config is applicable in both modes now: raw and JSON. It's described also in docs for pipeline tests.

There appears to be a slight difference in the configs for the raw and JSON modes.

Raw:

package-spec/versions/1/data_stream/_dev/test/pipeline/spec.yml

Lines 14 to 36 in 930b447

- description: Configuration of test case in raw format

type: file

pattern: '^test-[a-z0-9-]+\.log-config.json$'

contentMediaType: "application/json"

required: false

content:

type: object

additionalProperties: false

properties:

fields:

description: Field definitions

type: object

additionalProperties: true

dynamic_fields:

description: Dynamic fields with regular expressions defining their variable values.

type: object

multiline:

description: Multi-line configuration

type: object

additionalProperties: true

numeric_keyword_fields:

description: List of keyword type fields allowed to have a numeric value.

type: array

JSON:

package-spec/versions/1/data_stream/_dev/test/pipeline/spec.yml

Lines 37 to 55 in 930b447

- description: Configuration of test case in JSON format

type: file

pattern: '^test-[a-z0-9-]+\.json-config.json$'

contentMediaType: "application/json"

required: false

content:

type: object

additionalProperties: false

properties:

fields:

description: Field definitions

type: object

additionalProperties: true

dynamic_fields:

description: Dynamic fields with regular expressions defining their variable values.

type: object

numeric_keyword_fields:

description: List of keyword type fields allowed to have a numeric value.

type: array

It looks like the raw mode supports multiline whereas the JSON mode does not.

That's why I had to end up creating two different spec files: config_raw.spec.yml and config_json.spec.yml.

Ah, I see! In this case I think it's correct. Sorry about confusion.

* User volumes for service logs * Fixes * Bring back network connect * Address PR comments * WIP * Expose env as aliases * Fix

Add field to allow skipping of tests

57305f9

ycombinator requested a review from mtojek January 12, 2021 20:08

ycombinator added 3 commits January 12, 2021 12:09

Adding clarifying comment for file

caee22d

Adding period at end of entry.

c05767f

Adding CHANGELOG entry

b5cc3c8

ycombinator added 4 commits January 12, 2021 12:18

Adding test case for skipped system test configuration

93e5bf3

Updating spec

3626d34

Allow additional properties in system test config file

84d4570

Define data stream manifest file

f363add

ycombinator requested a review from adriansr January 12, 2021 20:38

ycombinator mentioned this pull request Jan 12, 2021

Allow tests to be disabled elastic/elastic-package#218

Closed

2 tasks

mtojek reviewed Jan 13, 2021

View reviewed changes

ycombinator added 3 commits January 13, 2021 11:32

Adding reason and link to skip object

687f9dc

Updating pipeline test config spec

81621ce

Add format: uri

eb0147c

ycombinator requested a review from mtojek January 13, 2021 20:22

Extract log-config separate file

10deb0b

Merge branch 'test-skip-fix-1' into test-skip

5e0361b

Adding spec for JSON config format

d77a22e

mtojek reviewed Jan 14, 2021

View reviewed changes

mtojek self-requested a review January 14, 2021 23:34

mtojek approved these changes Jan 14, 2021

View reviewed changes

ycombinator merged commit 24c262a into elastic:master Jan 14, 2021

ycombinator deleted the test-skip branch January 14, 2021 23:35

rw-access pushed a commit to rw-access/package-spec that referenced this pull request Mar 23, 2021

User volumes for service logs (elastic#105)

f5d9da1

* User volumes for service logs * Fixes * Bring back network connect * Address PR comments * WIP * Expose env as aliases * Fix

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add field to allow skipping of tests#105

Add field to allow skipping of tests#105
ycombinator merged 14 commits intoelastic:masterfrom
ycombinator:test-skip

ycombinator commented Jan 12, 2021 •

edited

Loading

Uh oh!

elasticmachine commented Jan 12, 2021 •

edited

Loading

Build stats

Uh oh!

mtojek left a comment

Uh oh!

ycombinator commented Jan 13, 2021

Uh oh!

ycombinator commented Jan 13, 2021 •

edited

Loading

Uh oh!

mtojek commented Jan 14, 2021

Uh oh!

ycombinator commented Jan 14, 2021 •

edited

Loading

Uh oh!

ycombinator commented Jan 14, 2021

Uh oh!

mtojek Jan 14, 2021

Uh oh!

ycombinator Jan 14, 2021

Uh oh!

mtojek Jan 14, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	- description: Configuration of test case in raw format
	type: file
	pattern: '^test-[a-z0-9-]+\.log-config.json$'
	contentMediaType: "application/json"
	required: false
	content:
	type: object
	additionalProperties: false
	properties:
	fields:
	description: Field definitions
	type: object
	additionalProperties: true
	dynamic_fields:
	description: Dynamic fields with regular expressions defining their variable values.
	type: object
	multiline:
	description: Multi-line configuration
	type: object
	additionalProperties: true
	numeric_keyword_fields:
	description: List of keyword type fields allowed to have a numeric value.
	type: array

	- description: Configuration of test case in JSON format
	type: file
	pattern: '^test-[a-z0-9-]+\.json-config.json$'
	contentMediaType: "application/json"
	required: false
	content:
	type: object
	additionalProperties: false
	properties:
	fields:
	description: Field definitions
	type: object
	additionalProperties: true
	dynamic_fields:
	description: Dynamic fields with regular expressions defining their variable values.
	type: object
	numeric_keyword_fields:
	description: List of keyword type fields allowed to have a numeric value.
	type: array

Conversation

ycombinator commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jan 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Build stats

Uh oh!

mtojek left a comment

Choose a reason for hiding this comment

Uh oh!

ycombinator commented Jan 13, 2021

Uh oh!

ycombinator commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mtojek commented Jan 14, 2021

Uh oh!

ycombinator commented Jan 14, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ycombinator commented Jan 14, 2021

Uh oh!

mtojek Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

ycombinator Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

mtojek Jan 14, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ycombinator commented Jan 12, 2021 •

edited

Loading

elasticmachine commented Jan 12, 2021 •

edited

Loading

ycombinator commented Jan 13, 2021 •

edited

Loading

ycombinator commented Jan 14, 2021 •

edited

Loading