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

Add livetest makefile command #78

Merged
merged 3 commits into from
Sep 26, 2022

Conversation

lucian-ioan
Copy link
Collaborator

@lucian-ioan lucian-ioan commented Aug 22, 2022

The live flag added in this test is used to get a live response from the GitHub API.
This might prove useful as sometimes the responses can change over time.

Let me know if this implementation is alright @endorama and I will continue writing it and update the rest of the tests with it.

@elasticmachine
Copy link
Collaborator

elasticmachine commented Aug 22, 2022

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2022-09-26T12:25:57.291+0000

  • Duration: 3 min 35 sec

Test stats 🧪

Test Results
Failed 0
Passed 72
Skipped 0
Total 72

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@endorama
Copy link
Member

endorama commented Sep 5, 2022

There is a way to do this without the flag: by deleting the YAML test file go-vcr is going to re run the test with live data.

My reason for not adding a flag is because even if it's a really cool Go feature is easy to get confused by it and go test just fails with an error not providing enough context (will just say flag not found). The implementation is very good, but I would address this differently.

Given that in any case we need to provide guidance in the docs about this, I would provide guidance on the file removal way, maybe through a make target that does it.

@lucian-ioan
Copy link
Collaborator Author

lucian-ioan commented Sep 23, 2022

@endorama I've added a makefile command that just deletes the testdata file in the fixtures folders.

In the original implementation I don't think the flag would have been an issue, given that it was optional. If the flag was not provided, the test would run using the recorded testdata.

The advantage of the make command is that after running it we can ensure all the future tests will be done with the new API responses, whereas the flag may or may not be used.

@lucian-ioan lucian-ioan marked this pull request as ready for review September 26, 2022 11:30
Copy link
Member

@endorama endorama left a comment

Choose a reason for hiding this comment

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

As this is a developer only change I would not include the changelog fragment.

Apart from that, lets merge this!

@lucian-ioan lucian-ioan changed the title Add github live flag to tests Add livetest makefile command Sep 26, 2022
@lucian-ioan lucian-ioan merged commit ddafc5a into elastic:main Sep 26, 2022
@lucian-ioan lucian-ioan deleted the github-live-flag-tests branch October 26, 2022 07:18
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.

3 participants