give user the option to update REDCap version in cypress.env.json

aldefouw / redcap_cypress_docker

Start here to work with REDCap Cypress Test Framework on a developer machine. This is a Developer Toolkit to install a test environment to run REDCap Cypress against an instance of REDCap.

MIT License

3 stars 7 forks source link

give user the option to update REDCap version in cypress.env.json #2

Closed ihabzee closed 1 week ago

ihabzee commented 5 months ago

Hi Adam,

Thanks for building this. It's a very useful repo. I made a minor change to the download_redcap.sh script so the user can update the REDCap version while downloading the folder. Please let me know if you have any questions.

Ihab.

aldefouw commented 2 months ago

Thanks @ihabzee.

Let me take a bit to think about this before approving - today is travel day for me so it might not be until later this week.

aldefouw commented 1 month ago

@ihabzee - I like this change overall, but there are a few things that I need fixed or clarified.

Why is the REDCap version entered twice?

https://github.com/aldefouw/redcap_cypress_docker/pull/2/commits/7829b004ea4968b4d751f7794e8239da1d99b909#diff-486ca128ea59a5d031ff5c729b7acef03dd5bcb85c01a3f4f5b7e5c98dab8751L3-R7

I assume that is just a mistake.

Can we write the JSON file to cypress.env.json.old or something instead of cypress.env.json-e ?

I wasn't initially sure where the overwritten file went.

Thanks for the contribution.

ihabzee commented 1 month ago

Hi @aldefouw I do not see the duplicate line. Its a mistake for sure.

Also, I added a line to rename the cypress.env.json-e to cypress.env.json-old.

Ihab.

aldefouw commented 1 month ago

@ihabzee: Sorry for the confusion.

You removed the duplicate line with a subsequent commit I had missed: https://github.com/aldefouw/redcap_cypress_docker/pull/2/commits/2603bb979f20ca653d000c814ad943981d01d849

I'll run some tests on this and then hopefully merge soon.

aldefouw commented 1 week ago

@ihabzee - Thanks for your input and sorry it has taken me so long to circle back on this! It has been a really busy time for me since REDCap Con.

I really like the spirit of this PR and your idea was shared by several other people (including those who didn't submit PRs but emailed me).

I ended up accepting another PR that was a simpler implementation. It was pointed out to me that the scenario where you download REDCap but do not change your REDCap version you're testing against is an edge case. We'll handle that edge case in a different way.

You can view the accepted changed in behavior in this commit .