-
Notifications
You must be signed in to change notification settings - Fork 14
Add e2e tests for static targets #30
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
Conversation
7d7a16b to
79f55c2
Compare
79f55c2 to
1ea0103
Compare
marcsanmi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| t.Setenv(configurator.LicenseKeyEnvKey, "") | ||
| t.Setenv(configurator.DataSourceNameEnvKey, "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was hitting an error since i configured the env vars to run the Tilt environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure I am following, are they colliding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test also covers the code where these env vars are expanded, if you have them in the session you are execute them , the test will fail (because of the expectations are checking other values for these).
I realize this "week" isolation because I exported this envars to run the e2e locally.
| scenarios: | ||
| - description: Scrape test environment metrics | ||
| before: | ||
| - cd ../../ && NR_PROM_LICENSE_KEY=${LICENSE_KEY} NR_PROM_CLUSTER=${SCENARIO_TAG} make tilt-ci |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, this is not that nice, could it be a step of the pipeline? How nri-kubernetes works without this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean the cd ../../ &&
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also below tilt down is way more clear and simple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agree is ugly, but is not complex i would say, is the simpler and clear solution i came up to actually re-use the make target and not duplicate bash scripts all over the repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like there is another way but i found it more obscure WDYT
https://superuser.com/questions/370575/how-to-run-make-file-from-any-directory
paologallinaharbur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just some doubts regarding the parser_test
|
golangci-lint is failing because the just released 1.48 version deprecated the |
Executes the e2e test action using the Tilt environment and checks for expected metrics that are in the test environment.
Pending on future PRs