-
Notifications
You must be signed in to change notification settings - Fork 40
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 integration tests for creating URL aliases #154
Comments
@Ansel-Hong thanks very much for taking this on. Here are some preliminary steps:
For this specific integration test, I would copy the In your assets directory, create a simple input CSV file (maybe using You will also need a configuration file in this assets directory. It will need to point to your input CSV (and should contain Once you have this all set up, you can run only your new test class using the following command (assumes your new test class is called Is this enough to get you going? If so, we can check in whenever you need more help. The test for the |
@mjordan Hi, can I get some tidbits as to how to find the node ID for the |
I'm tied up today but I'll provide a bit more info over the weekend. Thanks! |
https://github.com/mjordan/islandora_workbench/blob/main/tests/islandora_tests.py#L406 shows how I get the nids for a delete test - write them to a temporary file that has |
Ah makes sense, and for the |
Good question. Instead of merging, I would add some simple field data to the file I am writing out, for example:
|
Hi, sorry for taking so long to respond, just finished exams recently. I would like to ask why I'm getting a error message when I delete nodes saying |
Don't know... I haven't see that error before. Can you confirm that the nodes you are deleting have media attached? If so, I'll try to replicate the error. Also, what content type are your nodes? No sweat on the tests, thanks for checking in. |
The nodes Im deleting dont have media attached, they are of the "Article" type. Should I use another type instead? |
Type shouldn't matter, but I'll take a look. |
@Ansel-Hong I added a check for media that should avoid this problem. Can you pull down the latest changes to the main branch and test deleting again? |
Yup it works now thank you, I realised however the url alias is not deleted (under configuration>url alias) but I'm guessing thats the point of the integration test to check for issues like that. |
@mjordan I'm trying to push my local repo for issue-154 but im getting a error saying |
Looks like you are trying to push to my Github account. Also, I don't see a fork of Islandora Workbench at https://github.com/Ansel-Hong. I'm really only familiar with the workflow of forking a repo into your own Github account, pushing your changes to your fork, and then opening a pull request, which I would then merge into mine. Is what you are trying to do? |
Ah okay my bad, I dont know why it was in pushing with ssh. I managed to push it now. |
@Ansel-Hong feel free to open a pull request when you're ready. |
Okay I will do that still need to write code to remove the url alias during tearDown because right now it removes the nodes without deleting the corresponding url alias |
Should the test code only create a node with a url alias and update it without deleting? If so I can just send a pull request then. |
In general, the tests should delete everything they create. The one exception is taxonomy terms. If you want to open the pull request without deleting the aliases, go ahead. We can add code to delete those later. |
Okay Ill just make a pull request first so you can check if everything works on your end. Thanks, and sorry again for taking so long. |
For deleting the url alias shound I hook the url alias to the node or how do I delete it? |
@mjordan Sorry, forgot to @ you about the question, Thanks! |
No description provided.
The text was updated successfully, but these errors were encountered: