-
Notifications
You must be signed in to change notification settings - Fork 6
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
test: OIDC login workflow #19
base: master
Are you sure you want to change the base?
test: OIDC login workflow #19
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Hey @SohamRatnaparkhi, Can you remove commit docs: making docs verbose from this pr as it is not related to the issue this pr is solving! |
Yep, I tried removing that commit but I couldn't do so. Hence, I removed the changes in README file in the last commit (test: oicd auth tests added and README changes removed ) . I too wanted to remove that commit. Is there a way to do so. If not, then anyways merging this PR won't affect README. Thank you! |
Hey @SohamRatnaparkhi, you can maybe rebase the commit from the git history & then try to force push to the branch you are working on. You can see this for your ref: https://stackoverflow.com/questions/30893040/remove-commit-from-history |
ae8030e
to
73e1173
Compare
Hey @git-anurag-hub ,
Now the README changes commit doesn't exist. Kindly inform me if I have erred, so that I may rectify my mistakes and make another change. |
Hey @SohamRatnaparkhi, thanks for the change! Yeah now the commit is gone! Can you resolve the conflicts & also configure the Please ping in case of any help. Thanks for the pr again :) |
Hi, |
Hi, I noticed an issue related to failing tests that I recently encountered. Upon investigation, it seems that Cypress may be encountering difficulty in retrieving environment variables from the env file. As a result, when the host_uri in the Upon manually setting host_uri = localhost:3000 or manually setting host_uri to production link, the tests were able to execute successfully. I have also provided a video demonstration of this issue, in which I have used my personal LS password for testing purposes. Screencast.from.03-07-2023.01.10.06.PM.webm |
Solid work, @SohamRatnaparkhi! :) Any chance you can also fix the failing test on the CI machines? Or wait... is the "only" problem now that we don't have a service account that we can use to provide a username and password? |
Oops, seems like I had accidentally close the PR, my bad. Opened it again. Regarding the "test user", I see some problems coming up: I can imagine that LS AAI will not like to have a service account for that purpose. But I'll ask. Otherwise I could use mine. Either way, you'd need to be able to consume GitHub repo secrets in your test. Obviously we can't just commit a valid user/pass combination to the repo, even if it were an unprivileged service account. |
Yes @uniqueg , I will need to pull the dummy/service account password and username from the GitHub secrets. |
Hey @SohamRatnaparkhi, can you verify if without the test the application is working fine for you. Like all the login & other stuff is picking up the |
Hey @git-anurag-hub , I did some debugging and I can ensure you that the host_uri is being pulled correctly from env.development file in the the
Screencast.from.03-08-2023.04.50.28.PM.webmAnother point that I noticed while debugging is -
Video regarding the above conclusions with respect to Cypress. I have console logged the host uri that cypress recieves and you can see the results on right side of screen( I am highlighting them) Screencast for debugging Cypress |
@SohamRatnaparkhi: Cool. Ping me if you are done debugging with @git-anurag-hub and you figured out how to pull the GitHub secrets for consumption in your tests (maybe |
Hey @SohamRatnaparkhi, I just pulled your branch & everything seem to work fine on my end for the LS AAI login & I was able to login successfully! Can you stash your changes on this branch & check the functionality with having local HEAD of your brach synced with this remote branch! |
It is strange but without even stashing the changes, it worked on my end. I cleared the browser cache and then everything worked well. Now, how should we proceed regarding the |
Hi @git-anurag-hub and @uniqueg , This feature can be found here: Cypress environment variables We can use this as follows:
Now, in the test file: these variables can be accesses by I have tested this locally, and it works well there. |
Cool, sounds good :) Just FYI, ELIXIR/LS AAI are working on support for such test cases. Please check out the #aai channel on our Slack board for details (should be the last couple of messages in that channel). |
IMPORTANT: Please create an issue before filing a pull request! Changes need to be discussed before proceeding. Please read the contribution guidelines.
Details
Currently, there are no tests to check if some code breaks OIDC login workflow for Krini.
Refer to the corresponding issue with #1 for more information.
Testing
Appropriate tests have been written. Code passes all the test.
spec.cy.js.mp4
Documentation
No documentation required as it is not a feature upgrade.
Style
My code strongly adheres to the style convention of the project.
Closing issues
Closes #1