-
Notifications
You must be signed in to change notification settings - Fork 15
E2E test: torrent upload #185
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
E2E test: torrent upload #185
Conversation
It's only the first iteration. I'm using a fixed torrent file in the fixtures folder. I want to dynamically generate a new torrent file for each test execution so that I can isolate the test execution from previous executions. That was how we did it in the backend. With the current solution, you cannot execute the test twice because you cannot upload the torrent with the same infohash or title twice. We could remove the torrent after executing the tests, but the test could fail for many reasons that do not depend on the subject under the test. For example, the backend API could be temporarily not available. For those cases, if the torrent was already uploaded but not removed, I would like to be able to re-run the test. In the backend, we can run tests with a completely isolated environment but that's impossible here because we are using docker. In the backend, we just launch another child process accessing a different database. I will try to find a node package to build the torrent in memory and try to write the new torrent in the fixtures folder as described in the Cypress documentation: If that's not possible, we could try to use an API. You can also get files from APIs: https://docs.cypress.io/api/commands/selectfile#From-an-API-response Maybe we could create a new service with docker for the E2E test env with an API to generate torrent files. We could use the create-torrent package or the console command we are using in the backed: intermodal. But I will try first the previous solution because it's much simpler. |
I'm trying to generate the torrent file on-the-fly with a custom command like this:
But I cannot use callbacks because of a Cypress restriction. I've been discussing it with ChatGPT: https://chat.openai.com/share/99d2ebdc-613c-4a84-8281-bfc2fe82bc14 And there are some alternatives: Option 1Build our API service to generate torrent files on-the-fly. We can create a docker image for the service and add it to the docker composer configuration for the E2E testing environment. Option 2Add that feature to the application. I would work only for tiny files. But It needs to make sense for the end-user. It would be something like this: https://kimbatt.github.io/torrent-creator/ Pros
Cons:
Option 3Use an independent env for each test, like in the backend. So there is no need to generate random data. We could always use the pre-built fixtures in the repo.
I think it would be too slow with docker. Option 4Delete the torrent after running the test. Cons:
Pros:
ConclusionThe option suggested by ChatGPT seems to be easy to implement, and we do not need to add features to the app only for testing. Maybe it could be useful for other cases. LinksWhat do you think @da2ce7? |
Hello, I would like to ask/give an opinion. From what I see, I'm not sure if it would generate much noise to generate that file in the I'm not exactly sure how complex it can be to generate a torrent, but it seems like a better idea not to use Example using the
Finally, in the |
Hi @Wolfremium13 thank you for your feedback!
The reason I want to use a custom command is because I want to reuse the command in all tests that require upload a torrent. I want to have something like this:
If I tested it manually, the process would be:
That's how I did it for the API. See here. #[tokio::test]
async fn it_should_allow_authenticated_users_to_upload_new_torrents() {
let mut env = TestEnv::new();
env.start(api::Version::V1).await;
// ...
let uploader = new_logged_in_user(&env).await;
let client = Client::authenticated(&env.server_socket_addr().unwrap(), &uploader.token);
let test_torrent = random_torrent();
let info_hash = test_torrent.info_hash().clone();
let form: UploadTorrentMultipartForm = test_torrent.index_info.into();
let response = client.upload_torrent(form.into()).await;
let uploaded_torrent_response: UploadedTorrentResponse = serde_json::from_str(&response.body).unwrap();
assert_eq!(
uploaded_torrent_response.data.info_hash.to_lowercase(),
info_hash.to_lowercase()
);
assert!(response.is_json_and_ok());
} I do not want to generate the test torrent in the
I want to use
That would be my preferred solution, but I have not found a package that I can use with Cypress, which requires:
But since I only want to create very simple torrents, maybe I should explore this solution. I'm also considering another solution. In the backend, I'm using a console command to generate torrents: intermodal. CYpress has an exec command to run system command. Maybe I can do the same as I did in the backend. I had a similar problem; I could not find a library to create torrent files on the fly.
That example is precisely what I'm trying to achieve but I want to reuse this code in other tests:
Regarding using the torrent infohash in the assertion:
I wanted to check that the torrent is not only uploaded but also parsed correctly.
Yes, I should delete the newly generated files, but do you think I should also delete the torrent in the database? I prefer to reset the E2E test env from time to time because I have the impression that tests would be much slower if you have a lot of them. And finally, another thing to consider is that we do not need to create a temporary file. It seems you can
The final solution could be something like:
But moving the fixture creation to a custom command. But I would prefer your proposal, a torrent builder. I will try that solution first. |
Thanks for the feedback I appreciated a lot <3. I'll share a few opinions:
In my opinion yes, to keep it the use case enviroment as cleaned as possible (if we could do it). If for example we're getting radom numbers for the name, the probability to get exactly the same number it's low, but could happen and generate an unexpected behaviour on a test that shouldn't fail. Could happen with users created on the same instance of the database in different places.
Seems like your asstertion it's tied to the behaviour of the implementation of back-end in order to get the number. Maybe I'm wrong but, it could generate problems if for example we change the name or number of the file because it's random. |
Hey @Wolfremium13
OK. I'm not worried about the env on the CI because it's a new DB every time you run the tests, and there should not be collisions, however, leaving the DB clean I think it's a good thing and maybe not as slow as I expect. I can do it and remove it later of try to find another solution if tests are really slow.
The number is a domain ID, the torrent infohash which is a hash that does not change, but we are considering adding other protocols in the future (not only BitTorrent) and in that case, we could switch to use our ID, so I totally agree, maybe I only have to check in the details page that the title, description, etcetera are OK, even the infohash but not in the URL unless we consider that URL a permalink and we want to test that it's available or something like that. I mean, we should test it not because we want to be sure the torrent was uploaded correctly as I said, but for other reasons. For now, it's an implementation detail as you said. |
I think that creating a dynamic endpoint in the frontend something like Since we have the version namespace we can put it in Edit: This isn't just like a fixture. Dealing with torrents is a core-property of this application. |
Hi @da2ce7 I also think it's the most convenient. I think it's going to be used a lot for testing and maybe in the future it could be reused for a production feature, so I think it is worth the effort to build this new endpoint. And it should not be too difficult if we use a third-party library. |
Hi @Wolfremium13 @da2ce7, I've been thinking again about this problem and exploring the solution with a new
Conclusion
What do you think? I would like to consider all the alternatives before implementing the |
It will be used to generate an aidentifier for test torrents.
To identify the torrent, so that you do not need to parse resposne headers to get the filename or torrent identifier.
It contains info about the fixture torrent.
We remove the test torrent file after being used by the test.
We can merge this after merging torrust/torrust-index#237 The workflow fails because the new endpoint version to generate random test torrents has not been merged yet into develop in the backend. |
0f163cf refactor: invert the dependency between Torrent named constructors (Jose Celano) 4b6f25c test: add test for random torrent file generator service (Jose Celano) 40c4df0 refactor: extract hasher service (Jose Celano) b2870b9 refactor: extract torrent file service (Jose Celano) dfa260e fix: clippy warnings alter updating clippy to clippy 0.1.73 (Jose Celano) b269ecb feat!: change random torrent generator endpoint (Jose Celano) 30bf79e feat: new endpoint to generate random torrents (Jose Celano) dd1dc0c chore: add dependencies: hex, uuid (Jose Celano) Pull request description: Relates to: torrust/torrust-index-gui#185 Sometimes we need to generate a random torrent for testing purposes. We need to generate test torrents for the frontend application. With this new endpoint, you can get a random torrent: http://0.0.0.0:3001/v1/torrents/random The torrent is a single-file torrent using a UUID for its name and the original data file contents. In this repo, we use the `imdl` command to generate random torrents but in the frontend we are using cypress, and it seems the best option is [to make a request to an endpoint](https://docs.cypress.io/api/commands/request) to obtain dynamic fixtures like random (or customized in the future) torrents. torrust/torrust-index-gui#185 ### TODO - [x] Generate the random ID - [x] Calculate the correct value for the "pieces" field in the torrent file (sha1 hash of the contents). - [x] Refactor: extract service. Remove the domain code from the handler. - [ ] Refactor: add a new named the constructor for the `Torrent` instead of using the `from_db_info_files_and_announce_urls`. We do not need the `torrent_id`, for example. Other things that we could implement: - [ ] Add an env var to enable this endpoint only for development/testing environments. - [x] Add an URL parameter with the UUDI: `v1/torrents/random/:uuid`. We use the UUID for the torrent name (torrent file name: `name.torrent`). Top commit has no ACKs. Tree-SHA512: 3740a628b177303e60dac78969c786088c5c4391f7e1936fe6634031e533f38dea67a2aa7b876966456fb649f749773c43961d686c2c120ae62a2705be35c041
The workflow is passing because we've already merged the PR on the backend. The test works fine, and it's easy to read. We will probably extract a command to reuse the torrent generation and upload processes in other tests. But we can do it when we start working on adding other tests. I will continue working on this PR. I'm deleting the test torrent when the test ends, but I also want to delete the torrent in the database as @Wolfremium13 suggested here. |
In order to delete the temp test torrent when the test ends, I need to get the infohash so that I can run SQL query, but I do not want to get the infohash from the UI because I do not want to couple this cleaning part of the test to the UI. THe only way to get the infohash is from the original uploaded torrent file, but we have a bug on the backend. The infohash of the indexed torrent does not match the infohash of the originally uploaded torrent. I need to fix that issue before continuing. |
The file was being removed from the fixtures folder but not the torrent in the database. We are trying to clean the E2E env state after running tests. There are still some things pending to remove. For example, we create a new user for the test and the newly generated user is not removed.
Hey @da2ce7, In the end, I'm using a different approach. Instead of parsing the downloaded torrent file, I've added a custom HTTP header to the download response. This solution is faster because we do not need to parse the file. It has a side effect that I do not know if it is good. Since we are not parsing the file, we get the right infohash from the header even though the downloaded torrent has a different infohash. But in this test, we are not testing that the uploaded and downloaded torrents have the same infohash, we are only testing that you can upload a torrent, and after uploading it, you are redirected to the torrent details page. By the way, I have been unable to parse the torrent file with a Cypress task. Whenever I try to add a task that calls an async function, I get an error saying that the CYpress config file is invalid. You have to return a promise, but I can't make it work. But I really do not know why it was not working with the parse-torrent library. In fact, I'm using now an async function and it seems to work: The Cypress config file: import { defineConfig } from "cypress";
import { grantAdminRole } from "./cypress/e2e/contexts/user/tasks";
import { deleteTorrent } from "./cypress/e2e/contexts/torrent/tasks";
import { DatabaseConfig } from "./cypress/e2e/common/database";
function databaseConfig (config: Cypress.PluginConfigOptions): DatabaseConfig {
return {
filepath: config.env.db_file_path
};
}
export default defineConfig({
e2e: {
baseUrl: "http://localhost:3000",
setupNodeEvents (on, config) {
on("task", {
grantAdminRole: ({ username }) => {
return grantAdminRole(username, databaseConfig(config));
},
deleteTorrent: ({ infohash }) => {
return deleteTorrent(infohash, databaseConfig(config));
}
});
}
},
env: {
db_file_path: "./storage/database/torrust_index_backend_e2e_testing.db"
}
}); And the function of the task: // Custom tasks for user context
import { DatabaseConfig, DatabaseQuery, runDatabaseQuery } from "../../common/database";
// Task to grant admin role to a user by username
export const deleteTorrent = async (infohash: string, db_config: DatabaseConfig): Promise<boolean> => {
try {
await runDatabaseQuery(deleteTorrentQuery(infohash), db_config);
return true;
} catch (err) {
return await Promise.reject(err);
}
};
// Database query specifications
function deleteTorrentQuery (infohash: string): DatabaseQuery {
return {
query: "DELETE FROM torrust_torrents WHERE info_hash = ?",
params: [infohash]
};
} As you can see, The good thing is that while I was struggling to find a solution I took a look at the code inside the https://github.com/webtorrent/parse-torrent/blob/master/index.js#L118-L145 if (ArrayBuffer.isView(torrent)) {
torrent = bencode.decode(torrent)
}
// sanity check
ensure(torrent.info, 'info')
ensure(torrent.info['name.utf-8'] || torrent.info.name, 'info.name')
ensure(torrent.info['piece length'], 'info[\'piece length\']')
ensure(torrent.info.pieces, 'info.pieces')
if (torrent.info.files) {
torrent.info.files.forEach(file => {
ensure(typeof file.length === 'number', 'info.files[0].length')
ensure(file['path.utf-8'] || file.path, 'info.files[0].path')
})
} else {
ensure(typeof torrent.info.length === 'number', 'info.length')
}
const result = {
info: torrent.info,
infoBuffer: bencode.encode(torrent.info),
name: arr2text(torrent.info['name.utf-8'] || torrent.info.name),
announce: []
}
result.infoHashBuffer = await hash(result.infoBuffer)
result.infoHash = arr2hex(result.infoHashBuffer) I also tried to copy/paste that code instead of using the package, but I think there is a problem in this line:
I do not know how to create a Cypress task which calls to a function defined elsewhere which contains an This PR can be merged after emerging torrust/torrust-index#243 now even if the torrust/torrust-index#242 issue is not solved yet. |
… endpoints f7f76ff fix: clippy warning (Jose Celano) 414c6c1 feat: add a custom header with infohash to the download endpoints (Jose Celano) Pull request description: Endpoints, where you can download a torrent file, have now a custom header containing the torrent infohash. The name of the header is: `x-torrust-torrent-infohash`. It is added because [in the frontend we need the infohash in a cypress test](torrust/torrust-index-gui#185) and this way we do not need to parse the downloaded torrent which is faster and simpler. ACKs for top commit: da2ce7: ACK f7f76ff Tree-SHA512: 4be7ff27cbf96098a6e8d42f4f5064866bc0f8c5f02b58e5d70ed65c0cac17ecb1df9b3795d4b9e6af684dbd81ef1037cf1a056395e2d46bad0df7e109bfb26a
E2E test for torrent upload.
Strategy
I want to:
Current status
It works with a fixed torrent file fixture.
Example of the test passing on CI: https://github.com/torrust/torrust-index-frontend/actions/runs/5546141538/jobs/10126011522?pr=185#step:7:684
But the problem is I want to generate a random torrent file like in the backend to be able to run the test many times. If you try to rerun the same test, it will fail because the torrent already exists in the database.
In the backend, we solved it by using a console command to generate the torrent file. Here it's not possible because with Cypress, we cannot use:
Since it's run in the browser,
Screenshot