Skip to content

Implement static test runner#262

Merged
mtojek merged 20 commits intoelastic:masterfrom
mtojek:226-implement-static-test-runner
Mar 1, 2021
Merged

Implement static test runner#262
mtojek merged 20 commits intoelastic:masterfrom
mtojek:226-implement-static-test-runner

Conversation

@mtojek
Copy link
Copy Markdown
Contributor

@mtojek mtojek commented Feb 23, 2021

Fixes: #226

This PR implements static test runner with its first responsibility - linting sample events.

@mtojek mtojek self-assigned this Feb 23, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

elasticmachine commented Feb 23, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #262 updated

  • Start Time: 2021-03-01T14:18:02.162+0000

  • Duration: 19 min 30 sec

  • Commit: 8c0290f

Test stats 🧪

Test Results
Failed 0
Passed 311
Skipped 1
Total 312

Trends 🧪

Image of Build Times

Image of Tests

@mtojek mtojek marked this pull request as ready for review February 24, 2021 14:01
@mtojek mtojek requested a review from ycombinator February 24, 2021 14:01
@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Feb 24, 2021

I know that the CI is failing, but the code should be reviewable.

@ycombinator
Copy link
Copy Markdown
Contributor

This PR implements static test runner with its first responsibility - linting sample events.

Just curious, do you have other static assets in mind for follow up PRs?

}

func newConfig(staticTestFolderPath string) (*testConfig, error) {
configFilePath := filepath.Join(staticTestFolderPath, "config.yml")
Copy link
Copy Markdown
Contributor

@ycombinator ycombinator Feb 24, 2021

Choose a reason for hiding this comment

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

Should we define this as part of the package spec, as an optional file?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right, I totally forgot about the package-spec.

BTW Should I wait with this PR until you'll merge #260 or do you prefer the other way round?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

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.

I just merged #260 so you'll need to rebase this PR on top of master.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

Left a question and a minor typo fix suggestion.

@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Feb 25, 2021

Just curious, do you have other static assets in mind for follow up PRs?

Maybe not in follow ups, but we don't have any mechanism to verify/control what's inside screenshots/icons, e.g. size, resources.

Copy link
Copy Markdown
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

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

Left a couple minor doc language cleanup suggestions. After that, LGTM.

mtojek and others added 3 commits March 1, 2021 09:44
Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com>
Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com>
@mtojek
Copy link
Copy Markdown
Contributor Author

mtojek commented Mar 1, 2021

/test

@mtojek mtojek merged commit 9253ee3 into elastic:master Mar 1, 2021
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.

test asset: validate fields in sample event

3 participants